DOC-13478 vale review capella api #43

osfameron · 2025-10-08T12:43:00Z

I'll need to merge this in to get the "Vale review of OpenAPI files in Capella repo" feature deployed.

If you have time to check, let me know if you see any issues!
(No worry if not, I'll make good any regressions.)

These Views are customized to either: - `description` at top-level (Included) - `properties.*.description` This requires us to explicitly tell Vale which files are of which type in the .vale.ini like this example: ``` [cmd/cp-open-api/specs/schemas/alertIntegration/{BasicAuth,*AlertIntegration*,Exclude,Reque st*, Response*}.yaml] BasedOnStyles = Vale, write-good, proselint, Couchbase, Google View = OpenAPI [cmd/cp-open-api/specs/schemas/alertIntegration/{Token}.yaml] BasedOnStyles = Vale, write-good, proselint, Couchbase, Google View = OpenAPIInclude ``` (This restriction may go away with dasel 3, once Vale integrates that)

It looks like the behaviour of Vale with dasel view of a Yaml file gives inconsistent results for Line and Span. EXPECT: ====== the .Line and .Span records should point to the location in the file that the .Match text comes from. ACTUAL: ====== * For .adoc, this expectation is true. * For rules like Couchbase.VentilatedProse (theory: with `scope: raw`), we instead get the .Line indexed from the position of the Yaml token selected. For example, on the first line of the `.description`, it is marked as Line: 1 * For rules like Vale.Terms, we do get the correct .Line, but the .Span is indexed based on *YAML*'s understanding of the parsed content, not the textual source of the .yaml file contents.

sarahlwelton

Not sure how intelligent most of my comments are after like 10 minutes of research but lemme know I guess 😅

sarahlwelton · 2025-10-08T13:41:37Z

ValeStyles/.vale.ini


 attribute-missing = drop

+Vale.Spelling = NO


Curious why we don't want to run spellcheck?

Good question!
There were a lot of false positives, and it made testing very noisy.
I think the correct thing would be to add a vocabularies/CapellaOpenAPI.txt and update those such that there's a canonical list of words used. (Some of these might also go into our Couchbase.txt list, but I think it's worth keeping some hygiene between them.)

I'd like to do this as a subsequent step, rather than making it a blocker though.

Makes sense to me. Our current vocabularies aren't very fleshed out, so might be worth investing some time into those in general.

sarahlwelton · 2025-10-08T13:42:59Z

ValeStyles/.vale.ini

 BasedOnStyles = Vale, write-good, proselint, Couchbase, Google

-Vale.Spelling = NO
+[test/fixtures/test.yml]


I think this is only going to limit to files called test.yml. Is that going to work for where you want to deploy this? Shouldn't we just specify the yml file extension generally, instead?

Sorry, this is just a sample test.yml file, for the purpose of testing the infrastructure.
(It's actually proving that Vale+Views+Dasel Yaml mangles the line numbers... I've submitted a bug-report)

The customized .vale.ini then lives in the content repo, currently a POC one https://github.com/couchbaselabs/docs-vale-openapi-test-poc/blob/master/cmd/cp-open-api/specs/.vale.ini

That's because there's some mangling for specific files needing a specific view (again, to work around a Vale+Views+dasel bug/misfeature) which I don't think needs to go in the central .vale.ini.

Oh my, that .ini seems a little fragile - I hope that bug fix makes that easier to manage going forward.

sarahlwelton · 2025-10-08T13:45:01Z

ValeStyles/test/view-span.js

+  catch (err) {
+    console.log("Failed to run vale", err)
+  }


Damn, does Vale not give more specific errors to try and map onto, or is this just for a quick and dirty approach?

I'm not expecting the vale invocation to fail, so this just passes the error back to us so we can understand what happened (usually it'll be because I got the command parameters wrong).

sarahlwelton · 2025-10-08T13:46:04Z

ValeStyles/test/view-span.js

+describe('check spans for Asciidoc', function () {
+  testSpans('test/fixtures/basic.adoc')
+})


Is this just for testing? Just curious why we have a test for checking for AsciiDoc.

Ah, just:

Confirm that my expectation that .Line and .Span should identify the string in the source file actually works for .adoc

Having done that lets us know that the logic for checking the files is correct, and therefore a yaml failure is meaningful! (That's important because it's so easy to get off-by-one errors, I could easily have been checking the wrong text.)

Codify the .adoc expectation as a regression test so we notice if the Vale project ever breaks that understanding.

sarahlwelton · 2025-10-08T13:47:40Z

ValeStyles/config/views/OpenAPI.yml

+engine: dasel
+scopes:
+  - expr: properties.all().property(description?)
+    type: md


Does md just make linting this easier? Just curious.

OpenAPI descriptions are specified as Markdown I think?

sarahlwelton · 2025-10-08T13:48:50Z

ValeStyles/.vale.ini

-Vale.Spelling = NO
+[test/fixtures/test.yml]
+BasedOnStyles = Vale, write-good, proselint, Couchbase, Google
+View = OpenAPIInclude


Reading the Vale docs - you've only included one of the .yml files you added for defining a view? Does the other one not work?

It's the only view that's relevant for this file. See the POC .vale.ini linked above for more examples

sarahlwelton · 2025-10-08T13:53:59Z

ValeStyles/config/views/OpenAPI.yml

@@ -0,0 +1,4 @@
+engine: dasel
+scopes:
+  - expr: properties.all().property(description?)


Is this proper Dasel syntax? What are you trying to extract with this?

Yup. It's essentially .properties.*.description

e.g. https://github.com/couchbasecloud/couchbase-cloud/blob/main/cmd/cp-open-api/specs/schemas/backups/GetBackupByIDResponse.yaml#L22

sarahlwelton · 2025-10-08T13:54:35Z

ValeStyles/config/views/OpenAPIInclude.yml

@@ -0,0 +1,4 @@
+engine: dasel
+scopes:
+  - expr: description


So this might work for the very simple yml - do we want to make it more complex for the yml we're actually going to be linting? 🤔

This is an actual usecase!

https://github.com/couchbasecloud/couchbase-cloud/blob/main/cmd/cp-open-api/specs/schemas/backups/GetBackupByIDResponse.yaml is an example of OpenAPI.yml (.properties.*.description)

https://github.com/couchbasecloud/couchbase-cloud/blob/main/cmd/cp-open-api/specs/schemas/backups/Method.yaml is an example of this OpenAPIInclude.yml (.description)

The ideal would be expr: ..description which should be possible one Vale integrates the latest Dasel v3 with that feature added.

Ah, didn't realize that Vale hadn't quite caught up to the latest Dasel. Makes sense then.

osfameron added 2 commits September 10, 2025 10:53

osfameron requested review from Rebecca-Martinez007, TimLFletcher and sarahlwelton October 8, 2025 12:43

sarahlwelton reviewed Oct 8, 2025

View reviewed changes

osfameron merged commit 85c8e60 into main Oct 9, 2025
4 checks passed

DOC-13478 vale review capella api #43

DOC-13478 vale review capella api #43

Uh oh!

Conversation

osfameron commented Oct 8, 2025

Uh oh!

sarahlwelton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osfameron Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

osfameron Oct 8, 2025 •

edited

Loading