Repository navigation
Add a ProductChooser fed from the product registry - #417
eugenia-scandit wants to merge 13 commits into
Conversation
raffaelefarinaro
left a comment
There was a problem hiding this comment.
🤖 This review was AI-assisted. I verified the four blocking findings by hand against the branch and the failing gate run before posting.
Start with the copied block, because it is the cheapest fix and it clears CI. Lines 26 to 72 of docs/sdks/ios/sparkscan/intro.md reproduce the body of docs/partials/intro/_about-sparkscan.mdx, and line 73 still renders that partial. So the published page shows every section twice. It is also the entire reason the gate is red: "simply" on 45, 46 and 48, "e.g." on 48, and "overlayed" on 32 are all copied partial text. Deleting lines 26 to 72 fixes the double render and clears all four Vale errors in one move. Please do not spend time rewording "simply" and "e.g." in that block, it should not be there at all. The gate only lints changed docs, which is why the identical sentences pass in the untouched partial.
Second, the two registries do not agree on the product key, so the "can never drift" claim does not hold yet. docs-schema.yml constrains product to an enum of URL slugs, and two of them differ from the products.json keys: the schema allows matrixscan where the registry has matrixscan-batch, and label-capture where the registry has smart-label-capture. On those two intro pages the lookup misses, the heading degrades to "Choose this product if you want to", and the "Available on" row disappears with no error. URL_PRODUCT_MAPPING in src/components/utils/frameworks.ts already maps exactly those two cases and is used by parseSdksRoute, so importing it is the fix. Schema values id-bolt, express, core and platform have no registry entry at all.
Third, a single-item user_intents written as a YAML scalar takes the site build down, which blocks every other PR in the repo. It is latent, not live, and cheap to close. Details inline.
On reuse, FW_TO_PJ is a third copy of a mapping that already exists as FRAMEWORK_MAPPING in src/components/utils/frameworks.ts, and its .NET and React Native keys do not match what pages actually set. Details inline. Related but not yours to fix: framework: netIos (31 pages) and framework: react (30 pages) are not in the schema's framework enum, and the singular framework field is not validated at all. That is worth a separate ticket.
One more reuse note. FeatureList already renders a supported-framework list off the same frameworks map with the same n/a filter, as links labelled {framework} v{version}. The version is the useful part, since "available since 6.8" answers a question that "iOS" does not. Worth matching that instead of inert chips. Minor: the PR calls this a matrix, but it renders as a flat list of what is supported with no signal about what is not, so a reader cannot tell whether Titanium is unsupported or just unlisted.
Two things I would keep as they are: the empty-state guard correctly returns null only when all three inputs are empty while still rendering partial data, and isSupported handles the registry's conventions properly, including barcode-sequence where the version is real but apiUrl is "n/a".
Nothing here needs a redesign. The component is worth landing once the copied block is gone and the product lookup goes through the existing mapping.
…amework maps into one (#430) * fix(docs): validate the singular `framework` field and normalize its values docs-schema.yml defined the plural `frameworks` array with an enum but never defined the singular `framework` field at all. With additionalProperties: true it got no validation, and its values drifted. Canonical set: the enum is now the single source of truth for framework identifiers, and singular and plural share it - a page states one platform or several, never two different vocabularies. Normalized 117 pages: - netIos -> net-ios (31), netAndroid -> net-android (31), react -> react-native (30). Pure spelling drift; the canonical spellings already existed on other pages. - express (13) and bolt (12) were never frameworks. Those pages live under docs/hosted/, so they are `framework: hosted` with the value they were carrying moved to `product: express` / `product: id-bolt`, both of which already exist in the product enum. Two things the ticket's count could not have seen: - `kmp` is a sixth non-canonical value, on 37 pages - more than any entry in the ticket's table. It is not drift: those pages are the published /sdks/kmp/ tree, kmp is first-class in FRAMEWORK_MAPPING, QUERY_FRAMEWORK_TO_PATH and SkillsCallout, and being unreleased is already tracked by UNRELEASED_FRAMEWORK_SLUGS. The enum was stale, so kmp is added to it rather than the pages being rewritten. - FW_TO_PJ does not exist on main - PR #417 is unmerged. The real duplication is five maps, not three. Why nobody ever saw an error: docs-gate is a ratchet over changed files, so 37 pages violating the *plural* enum went unreported for months. Step 2 of the ticket alone would not have caught the other 117 either. scripts/verify-frameworks.cjs (yarn verify:frameworks, wired into CI) therefore checks the whole corpus, and the code maps as well: CONTENT no page may declare a framework outside the enum DRIFT no code map may key off a framework the enum does not define COVERAGE enum slugs missing from a map are reported as known gaps It found one live bug on its first run: FRAMEWORK_MAPPING had no `linux` entry, so parseSdksRoute() resolved no framework at all for every /sdks/linux/ page. Fixed here. Ticket acceptance test: a page set to `framework: netIos` fails the gate and passes after normalization - verified both ways. Step 4 (consolidating the five maps onto one) is deliberately NOT in this commit. Those maps have different membership, so merging them changes runtime behaviour - the linux gap above is exactly that kind of change - and it deserves its own reviewable diff. The DRIFT check above is what stops them diverging further in the meantime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(frameworks): one registry, every map derived from it Step 4 of the ticket. The framework set was written out five times - FRAMEWORK_MAPPING, QUERY_FRAMEWORK_TO_PATH and HOMEPAGE_FRAMEWORK_ALIASES in components/utils/frameworks.ts, plus FRAMEWORK_URL_PATH and FRAMEWORK_SLUG in SkillsCallout - each keyed slightly differently and nothing tying them together. That is why `linux` could be missing from one of them, and only that one, for the entire life of the page. src/constants/frameworks.ts is now the single registry. Each entry carries slug, display name, route segment, whether an Agent Skills page exists, whether it is unreleased, and any alternate spellings. All five maps are derived from it, as is UNRELEASED_FRAMEWORK_SLUGS. `slug` is the same string as the docs-schema.yml enum value, so the frontmatter vocabulary and the code vocabulary are now one vocabulary. yarn verify:frameworks compares the registry against the enum in both directions and fails on any difference - a slug in the registry the schema does not allow, or a slug the schema allows that no component can resolve. The risk in this refactor is not a crash, it is a map silently gaining or losing an entry, so scripts/test-frameworks.cjs (yarn test:frameworks, wired into CI) pins the expected membership of every derived map against what the hand-written maps held before. It loads the real modules through the TypeScript API rather than re-deriving anything, so it fails if the registry changes shape. 9 assertions, all passing, including the .NET two-segment route, the homepage aliases, and the Agent Skills exclusions. One deliberate membership change: `hosted` now counts as a framework without Agent Skills. It is not a homepage selector slug, so nothing passes it to frameworkHasAgentSkills - inert today, and correct if it ever is passed. Retargeting the gate also surfaced the same parser trap twice: anchoring on the first `{` or `[` after a name reads the TYPE annotation, not the value, and yields zero entries while reporting success. Both readers now anchor on the assignment and fail loudly on zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): skip prose checks when only frontmatter changed The ratchet checks a whole file as soon as a PR touches one line of it. That is right for prose someone is actually editing, and wrong for a mechanical metadata pass: normalizing `framework:` across 117 pages dragged in 233 pre-existing Vale findings the change neither caused nor altered (measured identical before and after: 233 either way). Nobody writes prose in frontmatter, so when a file's body is byte-identical to the ratchet base there is no prose to review. Those files now skip Vale and cspell. Schema and link checks still run on every changed file, and a file with any body edit is checked in full - verified by adding one sentence to one file and watching both new findings appear while the other 116 stayed skipped. Line endings are normalized before comparing: `git show` returns the repo blob with LF while the Windows working copy has CRLF, which made every file compare as changed and the skip silently never fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(frameworks): type the vocabulary, and check the data files against it Two gaps left by the registry refactor, both the same shape as the bug it fixed: a framework name that resolves to nothing, with no error. 1. THE REGISTRY WAS RUNTIME-ONLY. `slug: string` meant verify:frameworks guarded the frontmatter and the schema, but nothing guarded code. A component writing `frameworks: ['Web']` for `['web']` compiled fine and then matched no framework - exactly how `netIos` and `react` got into the frontmatter. `FrameworkSlug` closes that: 4 typo cases that used to compile are now errors, with "Did you mean" suggestions. The union is spelled out rather than derived. Deriving it needs `as const`, which turns FRAMEWORKS into 13 exact tuple members and drops the optional `aliases` / `unreleased` keys from the entries that omit them - `f.aliases` and `f.unreleased` stop typechecking. Verified: that route fails `tsc` in two consumers. So the union is a second copy, guarded the way the schema enum already is, by a new three-way check (union / registry / enum, every pair both directions). 2. products.json AND features.json WERE OUTSIDE THE REGISTRY. Both state per-framework availability keyed by DISPLAY name, so the slug enum cannot see them - a second vocabulary, unverified. It had already drifted. features.json carried `.Net iOS` and `.Net Android` against the registry's `.NET iOS` / `.NET Android`, 16 keys each. Nothing matched them: FeatureList looks up `feature.frameworks[currentFramework]` with a display name from the registry, so those 32 rows were unreachable. 32 keys normalized; the new DATA check keeps the two files inside the vocabulary. The check is one-directional on purpose: a product need not support every framework, so a registry display missing from a data file is fine. Only a name the registry does not know is an error. UNRELEASED_FRAMEWORK_SLUGS is annotated `string[]` explicitly. It is a membership-test array queried with raw route segments; narrowing it to FrameworkSlug makes `.includes(someString)` an error at its call site. Both new checks verified in both directions: - pre-fix features.json -> exit 1, naming both bad keys; post-fix exit 0 - union with `linux` renamed -> exit 1, reporting the extra and the missing Gates green: verify:frameworks, test:frameworks (9), tsc --noEmit. Unblocks #427, which adds `frameworks?: string[]` to AgentInstall - a sixth copy of the vocabulary landing just as this PR removes five. It can now import FrameworkSlug instead. Not fixed here, needs its own change: FeatureList derives the framework from `/sdks/([^/]+)/`, which yields `net` for /sdks/net/ios/ and matches no entry, so the feature table is empty on both .NET frameworks regardless of this normalization. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(frameworks): resolve /sdks/ paths through the registry, not a regex FeatureList detected the framework with `/\/sdks\/([^\/]+)\//` - a single path segment. For /sdks/net/ios/... that yields `net`, which is not a framework, so `FRAMEWORK_MAPPING['net']` was undefined and every feature was filtered out. The feature table has been empty on both .NET platforms for the life of those pages, and normalizing the features.json keys in the previous commit did not help: the lookup key never arrived. Formalized in the registry instead of patched in place. `routeSegment` already carries the two-segment shape (`net/ios`), so path resolution can be derived from it rather than restated: frameworkFromPath("/sdks/net/ios/add-sdk") -> net-ios / ".NET iOS" Matching is longest-`routeSegment`-first so a one-segment framework can never shadow a two-segment one, and it anchors on `sdks/` rather than the start of the path, so `/next/` and `/7.6.14/` prefixes work. A future multi-segment framework needs no change here - which is the difference between this and the regex it replaces. Measured end to end against features.json, counting features a .NET page would render: /sdks/net/ios/sparkscan/intro old: 0 new: 16 /sdks/net/android/matrixscan/intro old: 0 new: 16 /next/sdks/net/ios/add-sdk old: 0 new: 16 /sdks/ios/sparkscan/intro old: 16 new: 16 (unchanged) 4 new assertions: the two-segment routes, single-segment routes still resolving, version-prefixed paths, and that `/sdks/net/` alone resolves to nothing rather than to a framework. 13 passing. parseSdksRoute is deliberately left alone. It hardcodes `(?:net\/)?` in its regex, so it happens to be correct today; rewriting it onto frameworkFromPath would change what it returns for paths with no product segment, and there is no bug to justify that risk here. Gates: verify:frameworks, test:frameworks (13), tsc --noEmit, full build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(frameworks): one path parser, shared by both callers Two functions answered "which framework is this /sdks/ path" with two different regexes. FeatureList's captured a single segment and was wrong for .NET; parseSdksRoute hardcoded `(?:net\/)?` and then undid it with `.replace('/', '-')`, so it was right by coincidence of maintenance rather than by construction. Either could drift again, and one already had. frameworkFromRouteTail() in the registry is now the only place that maps a route segment to a framework. Both callers use it: frameworkFromPath() - unanchored, for a bare pathname (FeatureList) parseSdksRoute() - anchored, plus the product segment parseSdksRoute keeps every behaviour it had. Its regex now captures the whole tail and the registry resolves the framework from it, but the anchor stays (so /foo/sdks/ios/... is still not a route), the product segment stays mandatory (/sdks/ios/ is still {}), URL_PRODUCT_MAPPING still rewrites `matrixscan` and `label-capture`, and `lastSegment` is still absent rather than undefined when there is no third segment. Proved rather than asserted: the 15 inputs in PARSE_SDKS_ROUTE_BASELINE were captured from the old implementation before touching it, and are now compared with deepStrictEqual. A second test walks every routed framework and checks both parsers return the same one for the same path - the disagreement that caused the .NET bug cannot recur silently. Also derives DocItem's SKILL_LESS_FRAMEWORK_PREFIXES from `agentSkills` instead of listing ['/sdks/titanium/', '/sdks/linux/'] by hand. Same class of copy, and the registry already knows. Pinned to the previous literal. 16 assertions passing (was 13). Gates: verify:frameworks, test:frameworks, tsc --noEmit, full build. Known and left alone: DocItem matches those prefixes with pathname.startsWith(), so they miss version-prefixed paths like /next/sdks/titanium/. Pre-existing, unchanged here, and fixing it changes what the callout does on versioned pages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): hide the Agent Skills banner on versioned Titanium and Linux The callout was suppressed by startsWith() over ['/sdks/titanium/', '/sdks/linux/']. A docs-version segment defeats that: /next/sdks/titanium/... and /7.6.14/sdks/linux/... do not start with those strings, so the Agent Skills banner appeared on the two frameworks that have no Agent Skills page at all - on every version except the one served at the site root. Same class as the .NET FeatureList bug: a path matched by hand instead of through the registry. Now resolved with frameworkFromPath(), so the `agentSkills` flag decides and every docs version behaves the same: /sdks/titanium/core-concepts hidden (unchanged) /next/sdks/titanium/core-concepts hidden (was SHOWN) /7.6.14/sdks/linux/overview hidden (was SHOWN) /sdks/ios/add-sdk shown (unchanged) /next/sdks/net/ios/add-sdk shown (unchanged) This is a deliberate behaviour change, confirmed as the intended behaviour: a framework with no Agent Skills should never advertise them. The derived SKILL_LESS_FRAMEWORK_PREFIXES list is gone rather than fixed - there is nothing left to keep in step. Test extended to assert both directions across versioned paths. Gates: verify:frameworks, test:frameworks (16), tsc --noEmit, full build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(frameworks): guard the three remaining hand-written copies Five copies were collapsed onto the registry. Three were left, and a scan found them: FrameworksName, SearchBar's API_FRAMEWORK_LABELS, and useFrameworkItems' switcher list. They are guarded here rather than moved, because moving them changes behaviour and checking them cannot: - FrameworksName carries umbrella members (`net`, `xamarin`) that label grouping cards, not frameworks, and is keyed camelCase. It also indexes 9 components as FrameworksName[card.framework]. - SearchBar's keys are API-reference tokens (`dotnet.ios`) set by the generator, not by us. Its regex tokens are hand-tuned for match order and context, which mechanical derivation would lose. - useFrameworkItems must keep Xamarin: versioned_docs/version-6.28.11 and 7.6.14 still ship /sdks/xamarin/{ios,android,forms}, and the switcher shows them only on those versions. Dropping Xamarin would orphan real pages. Icons cannot move either - the registry is import-free so docusaurus.config.ts can read it under Node. So the same treatment the schema enum and the FrameworkSlug union already get: the copy stays, a gate compares it. Values must be registry display names; useFrameworkItems' `slug` must be a registry routeSegment. Legacy Xamarin routes and the umbrella displays are named exemptions, not silence. Reading is scoped, not file-wide: API_FRAMEWORK_LABELS is located by name and read by brace matching, because a line-level scan over SearchBar also picks up `interaction: "mouse"` and the `fw: "net/ios"` query tokens as if they were framework names. A renamed or reshaped list reports "unchecked" rather than passing empty - verified by renaming the const. Verified in both directions. Broke one value in each of the three files: frameworksName.ts display name "iOSS" is not in the registry SearchBar/index.js display name "Webb" is not in the registry useFrameworkItems label "Linuxx" is not in the registry useFrameworkItems route "linuks" is not in the registry exit 1 with all four, exit 0 restored. No drift exists today. Gates: verify:frameworks, test:frameworks (16), tsc --noEmit, full build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs): close the holes review found in the framework gate Six findings, four of them ways the new gate could be defeated or could fail on valid input. - `framework: "ios"` FAILED the gate: the value regex captured the quotes and reported `framework ""ios"" is not in the enum`. Valid YAML, accepted by docs-schema.yml, and this repo quotes frontmatter scalars widely, so the next author to quote this field would have turned build-docs red. `framework: ios # note` was worse - skipped, not flagged. - The plural `frameworks:` field was barely checked. `fm` is sliced before the closing `---` so the last line has no newline, while the item pattern required one: a one-item list was skipped entirely and a two-item list lost its last item. An inline `[ios, bogus]` matched nothing. No page uses the field yet, so none of this was exercised. All four shapes now report, and a shape the parser cannot read is reported rather than passed. - The header claimed "the whole corpus" and meant docs/ only. versioned_docs/ holds ~693 declarations, ~470 in the pre-rename vocabulary (`react` x60, `netIos` x59, `xamarinForms`, `express`). Rewriting frozen releases' frontmatter to satisfy a gate is not the answer, so the exemption is now stated in the script and in docs-schema.yml, which made the same wrong claim. - docs-gate's frontmatter-only skip switched off cspell as well as Vale, but `description` and `title` are prose - frontmatter.cjs runs anti-fluff checks on `description` and cspell.json has no frontmatter exclusion. A PR rewriting only a description shipped its typos unchecked. cspell now runs on every changed file; only Vale is skipped. - The two path parsers disagreed on prefixes, and PR previews hit it. Previews build with base_url=/data-capture-documentation/pr-preview/pr-N/ and useLocation().pathname includes it, so the anchored parseSdksRoute returned {} on every preview page: DocItem saw no product page and fell through to the shared Agent Skills callout, which defaults to iOS for every framework - while frameworkFromPath resolved the same path fine. The test that asserted `/foo/sdks/ios/x/y` -> {} was pinning the divergence in; it now asserts the route, with two real preview paths alongside. - isIdBoltPage kept the startsWith the hunk above it was rewritten to remove, so it failed on the same preview paths and would fail on an ID Bolt snapshot. Verified: 10 frontmatter shapes behave correctly (quoted, single-quoted, commented, bogus-quoted, one-item list, last-item, inline, unreadable, valid); parseSdksRoute 6/6 including two preview paths; isIdBoltPage 7/7; verify:frameworks OK; test:frameworks 16 passed; tsc clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs): parse the frontmatter with a parser Review round on this branch, and the honest summary is that hand-parsing YAML cost three rounds of holes for a benefit that did not exist. The docblock claimed the fields were hand-parsed "rather than run through js-yaml, because this gate walks 676 files on every CI run". Two things wrong with that: this file already `require`s js-yaml at line 56, and parsing all 676 files with it takes 60 ms. What the regexes bought instead was a series of legal YAML shapes the gate either failed on or, worse, passed in silence: `framework:` with the value on the NEXT line passed unchecked, and the unreadable-shape sentinel covered only the PLURAL key - so the singular field this branch is named for could be defeated by pressing Enter. A `frameworks:` block list failed on CRLF, which is every file in this checkout, and on a blank line or a comment line between the key and its items. The error told the author to use a block list, which is what they had written. No page uses the plural field today, so CI stayed green and none of it was ever exercised against real content - and `yarn verify:frameworks` is a build-docs step. `framework: "ios" # note` - quotes AND a trailing comment - failed the build, because comment-stripping was skipped for quoted values and the unquote regex then required the value to end with its quote. `declaredFrameworks` now calls `yaml.load` on the frontmatter block. None of those shapes needs handling, because the parser handles them by being the parser. A value that is not a string still reports rather than coerces, so `framework: [ios]` or a number is flagged. Verified by writing each shape as a real page under docs/ and running the real gate: 34 cases, LF and CRLF, all correct - including the next-line value, accepted when valid and flagged when bogus. THE FILE WAS BINARY TO GIT. `UNREADABLE` was a raw NUL byte, so this 486-line gate showed up in its own PR as `Bin 0 -> 19236 bytes` - unreviewable - and `git grep` answered "Binary file ... matches" instead of the line. Written as the escape `"\0unreadable"` now; `git grep` finds it again. A missing invariant is now checked: an entry with `agentSkills: true` must have a `routeSegment`. Nothing verified it, and both consumers cast the null away (`routeSegment as string` in utils/frameworks.ts and SkillsCallout), so such an entry would silently build `/sdks/undefined/agent-skills`. Not hypothetical - `hosted` is the entry with `routeSegment: null` and skills.json already has an id-bolt skill. Proven to fire: flipping `hosted.agentSkills` makes the gate exit 1. Smaller corrections, three of them to comments that had become false: docs-gate printed one `fatal: path ... exists on disk, but not in ...` line per file a PR ADDS, because `git show <base>:<newfile>` always fails and sh() inherits git's stderr on purpose. Forty new pages meant forty fatal lines before the gate said anything, reading as a crash. A quiet variant is used for that one lookup, where failure is expected and meaningless. Measured: 0 such lines now. `frameworkFromPath`'s docblock still told the reader to use `parseSdksRoute` "when a path outside /sdks/ must not match" - but this branch unanchored parseSdksRoute for the same reason, and test-frameworks.cjs now pins `/foo/sdks/ios/x/y` as resolving. Neither anchors any more. docs-schema.yml pointed at src/components/utils/frameworks.ts as the file that must cover the slug set. The gate reads src/constants/frameworks.ts; the utils one is derived and never read, so adding a slug where the comment pointed satisfied nothing. unreleasedFrameworks.ts said it was "kept free of imports" three lines above the import this branch added. Restated as the real constraint: nothing on its import chain may use a webpack alias, because docusaurus.config.ts is loaded by Node. Checked against the 8.6 release this branch merged: nothing here assumes 8.5.3 is served or that its snapshot exists. verify-frameworks 676 docs / 409 declaring, test-frameworks 16/16, docs-gate clean, tsc clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs): state the real constraint on unreleasedFrameworks' imports The header said the file was "kept free of imports so docusaurus.config.ts can read it too" three lines above the `import { FRAMEWORKS }` this branch added. The constraint it was protecting is narrower and still real: nothing on this file's import chain may use a webpack alias, because Node loads the config directly and cannot resolve one. That holds today only because ./frameworks happens to be alias-free, and nothing guards it - so the note now says which property to keep rather than one the file no longer has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs): stop the frontmatter-only skip from disabling a blocking check The review returned a no-go on one item, and it was a regression this branch introduced rather than a hole in a new guard. VALE LINTS FRONTMATTER, so skipping a metadata-only change wholesale turned off a build-blocking prose check for exactly the workflow the skip was built for. A banned word in `description` is a Severity: error alert, and .vale.ini sets MinAlertLevel = error - but with the skip, a description reading "scans identity documents effortlessly and obviously" gave `0 error(s)`, where the same words in the body fail the build. Neither word is in frontmatter.cjs's FLUFF_WORDS and cspell only catches misspellings, so nothing covered them. The hunk's own comment conceded "`description` and `title` ARE prose" two lines after asserting nobody writes prose in frontmatter; the second half was the true one. Every changed file goes to Vale again. For the metadata-only ones the alerts are capped at the frontmatter's closing `---`, so a rewritten description is checked while untouched body prose is not. Verified both directions with the review's own repro: banned words in the description alone now exit 1 naming both, and a clean description on the same file charges 0 body alerts. The five non-blocking findings are fixed too, each proven with a probe rather than argued: A UTF-8 BOM defeated the whole-corpus check completely - `startsWith("---")` is false with U+FEFF, so the function returned [] and a page could declare any framework while the gate printed OK. gray-matter strips the BOM, so Docusaurus renders such a page normally and nothing else would have noticed. Stripped now, and the open is anchored `^---\r?\n` like frontmatter.cjs. Every registry reader hardcoded double quotes. A single-quoted entry was invisible - an unknown `unity` slug in single quotes passed - and mixed quoting produced a FALSE failure naming a missing routeSegment the entry had. No formatter is configured in this repo and the file this registry replaced used single quotes, so that style is live. All four regexes take `['"]` now. src/data/skills.json was a third display-name-keyed data file and was not checked at all. It is an object, not a list, so it needed shape handling. A typo there leaves the product callout undefined and the framework silently vanishes from the Skills page. Now 3 data files, 31 names; a `.Net iOS Typo` fires exit 1. `frameworks: []`, `framework:` with no value, `framework: ~` and a null list item all passed. docs-schema.yml forbids them, but frontmatter.cjs only sees files a PR touched - which is the gap this gate exists to close. walk() had no `_`-prefix exclusion, so a partial opening with a `---` thematic break was reported as "frontmatter does not parse as YAML" - the wrong gate named for a file with no frontmatter. Partials are excluded as in docs-gate, and the scan is 616 pages rather than 676. One caveat the review raised that is worth carrying: this checkout has js-yaml 4.1.0 while package.json declares ^5.2.2 and yarn.lock pins 5.2.3, so my runs exercised v4. The review diffed both against all 616 frontmatter blocks and found 0 differences; the only load-time change is merge keys (`<<`), which appear nowhere in docs/. verify-frameworks exit 0 (616 docs, 409 declaring), test-frameworks 16/16, docs-gate clean, tsc clean, and both probe suites - 34 frontmatter shapes across LF and CRLF, plus the BOM/empty/null/quoting/partial cases - pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs): make the frontmatter cap fail closed, and stop two silent passes The review returned Go on the previous commit and listed seven items. None blocked, but three are the same silent-pass class this branch exists to remove, so they are fixed rather than deferred. TWO SILENT PASSES IN THE WHOLE-CORPUS GATE: An opening `---` with no closing one read as "declares no framework". A page with `framework: unity` and no closing fence printed OK and exited 0 - the exact silent pass the UNREADABLE sentinel three lines below exists to stop. It now reports. Check 4 passed with zero names when a data file changed shape, because an empty Set is truthy and the guard was `if (!names)`. Renaming skills.json's `frameworks` key to `platforms` left the gate printing "3 checked" and OK. Every sibling check fails loudly on parsing zero entries; this one no longer is the exception. Its error text also said "missing or not an array", which stopped being true when the object shape was added. THE FRONTMATTER CAP NOW FAILS CLOSED. `frontmatterEndLine` matched `---` exactly, but `bodyOf` and frontmatter.cjs both accept a trailing space - so a page whose closing fence was `--- ` returned 0, which is indistinguishable from "no frontmatter", and the file was charged for its entire untouched body. That is the 233-alert failure the skip was built to avoid, arriving through a space. Trailing whitespace is now accepted, and an unterminated frontmatter returns -1, which leaves the file out of Vale entirely rather than guessing an extent: the schema check already reports the malformed frontmatter itself. `bodyOf` also never stripped the BOM, though this branch added BOM handling in two other places, so a frontmatter-only edit to a BOM'd page could not qualify for the skip at all. Two comments and a pin: The log said "skipping Vale for them" about exactly the files Vale now runs on. Someone debugging a `vale:` finding on a metadata-only PR was told on the line above that Vale had been skipped. The cap is the whole frontmatter, not the frontmatter lines the diff touched, so a mechanical `framework:` pass is answerable for a pre-existing alert elsewhere in the same frontmatter. That is a deliberate trade - line-level attribution needs the diff hunks, and the alternative is the skip that disabled the check - and it is now stated where the cap is defined, with the one page corpus-wide that hits it named. CI pinned Vale 3.7.1 while the cap was verified on 3.15.1. The whole mechanism rests on Vale attributing a `description` alert to its true file line, and a local pass on one version proves nothing about another. Pinned to 3.15.1. Proven rather than argued: the unterminated page now reports, the renamed skills.json keys now report, and frontmatterEndLine returns 3 / 3 / 0 / -1 / 3 for a trailing-space close, an exact close, no frontmatter, an unterminated fence and a BOM'd close, with bodyOf stripping the BOM. verify-frameworks exit 0, docs-gate clean, and the earlier suites - 34 frontmatter shapes across LF and CRLF, and the BOM/empty/null/quoting/partial cases - still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): report per-key data-file misses, keep odd fences in Vale Two of these are mine from c28f18e77, and both were claims that did not hold when probed properly. The zero-names guard did not catch a renamed skills.json key. `frameworks` and `products` were unioned into one Set, so renaming only `frameworks` still resolved names through products.* - 31 of them, the same count as before - and the check printed OK. My earlier probe renamed both keys, which is the only reason it passed. Checked per key now: renaming one reports it and exits 1, while the total still reads 31, which is exactly why a count could not show it. The new terminator test was stricter than every other fence reader here. bodyOf uses indexOf("\n---"), frontmatter.cjs uses /\r?\n---/, and gray-matter accepts any ---prefixed line, so a `----` fence parses everywhere except my equality test - which returned -1 and dropped the file out of Vale entirely. A banned word in such a page's description exited 0, where before this branch it was caught. startsWith on both fences now; verified that `----`, `-----` and a trailing space all yield a real extent, and that `seamless` in a description is reported at its true line with the cap in force. Four smaller ones: - UNTERMINATED is its own sentinel. An opening fence with no closing one never reached js-yaml, so "does not parse as YAML" pointed the author at a syntax error that does not exist. - pagesWithField counts only real declarations. A page reported for a frontmatter-level problem declares nothing, and counting it inflated "N declare a framework" with pages that may declare none. - The log names the files the -1 branch leaves out, instead of claiming Vale ran capped on every metadata-only file. - frontmatter.cjs strips the BOM - the fourth of four fence readers to agree. Without it a BOM'd page with valid frontmatter was reported as "missing or invalid": fail closed, but naming the wrong problem. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): close the per-item data-file hole, test the gate readers The per-key fix in 5eda36fb3 left the identical hole open one function away. skills.json is an object of maps and is now checked per map; products.json and features.json are ARRAYS of items, and there `if (!fw) continue` read a row that declares no frameworks as nothing to check. So renaming one of features.json's 16 `frameworks` keys, or emptying one products.json item's map, left the other rows supplying the union, the total still reading 31, and the gate printing OK: src/data/features.json: entry "SerialNumberBarcode" is absent or empty src/data/products.json: entry "barcode-capture" is absent or empty Both now exit 1 and name the entry, while the count still reads 31 - which is the whole point, because a count cannot show a part going missing. Every item in both files carries the key today, so an absent one is a shape change rather than an exemption; the comment says so, so a future exemption has to be a decision. Three more silent passes, each reproduced before and after: - The agentSkills/routeSegment invariant was matched with `\{[^{}]*slug:...\}`, which cannot match an entry containing a nested object. Adding `meta: { a: 1 }` to the `hosted` entry dropped it from the only invariant here guarding a runtime URL bug (/sdks/undefined/agent-skills). Brace-matched now, via registryEntries(), and asserted against the `slug` count - so an entry the matcher cannot see is loud instead of free. - readList and readObjectValues were double-quote-only, so `ios = 'iOS'` or `label: 'iOS'` in any of the three UI copies went unchecked. The miss is partial, so the zero-entries guard never fired. Both accept `['"]` now, as registryValues already did and documents why. The enum's trailing comma is optional too: dropping it on the final member is valid TS and exempted that member. - An opening fence with trailing whitespace. gray-matter accepts `--- ` and `---\t`, so Docusaurus honours such a page's `framework:` while declaredFrameworks skipped it in silence and frontmatter.cjs reported a valid page as having no frontmatter. Both use `/^---[ \t]*\r?\n/` now. This is the same argument the BOM fix already accepted, just not applied to whitespace. And the reason this is round five: none of the three gate scripts had a single test. test-frameworks.cjs pins the registry and the path parsers; every defect found in five rounds lived in the readers instead. scripts/test-docs-gate.cjs (10 tests, wired into build-docs.yml) is table-driven over the shapes that defeated them - 23 frontmatter shapes, 10 fence shapes, 8 data-file shapes, 6 quoting shapes. I verified the tests bite by reverting each fix in place and confirming the matching test fails, then restoring. readList and readObjectValues moved to module scope to be reachable from a test; they were closures inside main(), which is why the quoting hole went untested. dataFileFrameworkNames returns { names, missing } or { error } instead of a Set with a property bolted on, and JSON.parse and the registry reads are guarded, so a malformed file gets one of this file's own sentences rather than a stack trace. Two comment corrections. frontmatterEndLine claimed frontmatter.cjs accepts a trailing space on the opening fence (it did not, until this commit) and that gray-matter accepts any `---`-prefixed OPENING fence (it does not - that holds for the closing fence only). The `startsWith` on the opening fence is deliberately looser than gray-matter, and the comment now says why: bodyOf makes the identical call, so the cap and the metadata-only decision agree on every input, which is the property the cap actually rests on. Also recorded that the `-1` branch needs the BASE blob to be unterminated, and that what the cap preserves is bounded by what Vale reports - measured, it flags a banned word in a single-line description, in a literal block scalar and in title, but not in a folded `>-` scalar or a keywords list item. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): per-product granularity, and tests that actually pin the fixes Two things, and the second is the reason there was a sixth round. The one remaining silent pass. dataFileFrameworkNames has now produced the same hole at three granularities in three consecutive rounds: the top-level key, the array item, and - still open until this commit - the individual product. All 8 of skills.json's products were folded into one Set, so emptying one of them left the other seven supplying names, the total still reading 31, and the gate printing OK. That is exactly the failure the function's own comment names: an empty product map leaves productSkills?.[framework] undefined in SkillsCallout, so the callout returns null and vanishes for every framework of that product. Reported per product now, empty or non-object only - matrixscan-pick lists two frameworks today and varying content stays legitimate. And the tests from the last commit did not pin what they claimed. Reverting the brace-matching fix, either readList quoting regex, or frontmatter.cjs's fence each left 10/10 green, because the suite read the REAL registry (which has no nested object) and retyped the production regexes as test literals. A test that passes with the bug reverted pins nothing. So: - Every reader takes its source text as an argument, and every case drives it with a fixture. The three call-site regexes are gone entirely - the readers own their patterns now, so there is nothing left to retype. - readList/readObjectValues are replaced by readers that report what they could not read: enumMemberValues, entryFieldValues, objectLiteralValues. A partial miss used to be free, because `found.length` stayed non-zero: `linux = ""` and `linux = LINUX_DISPLAY` each left one member unchecked while the gate said OK. Both now exit 1 and name the member. - Quotes are MATCHED, not a character class at each end. `['"]([^'"]+)['"]` truncated `label: "iOS's Legacy"` at the apostrophe and checked the truncation - which is a valid display name, so the gate passed a value it never saw whole. QUOTED uses named backreferences, because a numbered one shifts when concatenated after another group; getting that wrong once made \1 point at the member name. - arrayEntries and registryValues share one span finder (arrayBody), so the registry's entries and its `slug` count are read the same way - which is what makes the count assertion between them meaningful. Comments are stripped quote-aware first: the switcher's array carries a comment mentioning `${linkVersion}/${slug}`, and counting those braces produced phantom entries. - `agentSkills\s*:\s*true`. A space before the colon skipped the entry without tripping the count assertion, since the entry was still read. Same file, same no-formatter argument as the quoting fixes. The suite is 13 tests and calls process.chdir(ROOT), so it no longer reports a spurious failure when run from another directory; its scratch dir is pid-suffixed and gitignored. I verified all seven fixes bite by reverting each in place and confirming the matching test fails - including the three the previous suite missed. src/components/utils/frameworks.ts: the `routeSegment === null` branch is unreachable today, since frameworkFromRouteTail searches only the routed frameworks. Kept, with a comment saying so: the next line dereferences routeSegment, so if the routed set ever widens the alternative to this branch is a runtime throw rather than a type error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(docs-gate): make this branch merge cleanly alongside #429 No behaviour change on this branch - the gate's output is byte-identical to before this commit. It exists so #429 and #430 can land in either order without a hand resolution, and without either branch carrying the other's change. #429 and #430 both rewrite the same two regions, and `git merge-tree` reported a conflict in `.github/workflows/docs-gate.yml` and `scripts/docs-gate/index.cjs`. Three moves remove it: - `pagesOnly()` is defined and used at the schema and link call sites. On THIS branch it filters nothing, because changedDocs still excludes `_`-prefixed files from the list - it is a no-op here and load-bearing on #429, which widens that list so partials reach Vale and cspell. The definition and the two call sites are byte-identical on both branches, so git merges them as one change rather than as two competing ones. Its comment says only what is true on both branches: why the structural checks must not see partials. #429's measured consequences of widening the file list stay beside that change, where they are true. - `lastRatchetBase` is assigned next to the base resolution it comes from instead of beside the file-list predicate, and bodyOf/frontmatterOnly moved below findVale. #429's one remaining unique edit in this file is that predicate line, and nothing here now sits within git's three lines of context of it. - The Vale pin comment is the union of both branches' reasons. Both pin 3.15.1 for reasons that are both true - the Scandit rules were measured on it, and the frontmatter cap depends on Vale attributing a `description` alert to its true file line - so one text serves both and that hunk stops competing. Fixed its first line's indentation while there, which was six spaces where the rest of the block is ten. Verified: gate output diffed identical against the pre-commit run; test:docs-gate 13 passed, test:frameworks 16 passed, verify:frameworks OK, tsc clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): read only the top level, and pin the reporting steps Two silent passes, both in readers the previous commit introduced, and both the same mistake: a regex over a whole span, when only its top level is the thing being described. entryFieldValues searched the entire entry text, nested objects included. So `{ label: "Linux", meta: { slug: "linux" } }` had no top-level `slug` and the nested one answered for it - useFrameworkItems then builds `${linkVersion}/undefined/add-sdk` for that switcher item, and the gate printed OK. Doubly awkward, because arrayEntries' brace matching exists precisely so an entry carrying `meta: { ... }` is read whole, and then the field reader mis-read it. objectLiteralValues split the literal body on plain commas and tested for a quoted string anywhere in the chunk. A key whose value is a nested object kept that object's commas, so the chunk still held a quoted string, the key was reported as fine, and the NESTED string is what got checked against the registry. Reproduced on the real SearchBar map: rewriting `linux: "Linux"` as `linux: { label: "Linux" }` exited 0, and so did replacing it with a spread. Both now go through topLevelOnly/topLevelPairs, which blank nested spans and split only at commas that are at depth 0 and outside a string. That also removes a false report - `a: "x, y"` used to be flagged as having no string value - and entryFieldValues reports a field declared twice rather than taking the first, since the later one wins at runtime. And the reason those two could ship: nothing tested that anything REPORTS what the readers find. Review deleted five separate reporting decisions in main() - the uiErrors missing loop, the data-file missing loop, the entry-count assertion, and both halves of the whitespace-tolerant agentSkills scan - and the suite stayed green 13/13 each time. The readers' `missing` arrays were well covered; the code that turns them into errors was not covered at all. So uiCopyErrors, dataFileErrors and registryInvariantErrors are pure functions now, pinned by their messages. All five deletions fail a test. stripComments is a single-pass scanner with three states, because both of its shortcuts blanked live code. Block comments were stripped by a regex before any quote awareness, so an opening marker inside a string ate to the next closing one. And a regex literal ending in an escaped slash read as a `//` comment: 163 characters of live code in SearchBar, measured against TypeScript's own comment ranges. `{` and `}` are deliberately not regex-opening characters here - the file is JSX, and `{...config} />` puts a slash right after a brace, which swallowed the rest of the file including real comments. If the scan still ends inside a string or a regex the function returns null and the callers report the file unchecked, which is loud rather than quietly mis-read. Verified by transpiling original and stripped text with removeComments and diffing: the program is identical on all four files it reads, and stripping is idempotent. Four smaller ones: - declStart anchors on a word boundary. `indexOf("const FRAMEWORKS")` is a prefix match, so a `const FRAMEWORKS_ORDER` declared first would have been read instead - loud for the registry, silent for the switcher. - enumMemberValues splits on top-level commas rather than on lines. A one-line enum matched no member, so the first was reported and the rest dropped. - unionSlugs is quote-matched, the last reader still breaking the rule this file states. - enumSlugs returns its problems instead of throwing. A vocabulary mismatch gave a Node stack trace, which is the presentation dataFileFrameworkNames was changed to avoid. Suite is 23 tests. Every fix above was verified to bite by reverting it in place and confirming the matching test fails. The field under test is stated in the entryFieldValues table rather than inferred from the fixture text, and the frontmatter fence case uses a temp dir outside the repo since it only needs an absolute path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): read the top level in the two places that still didn't, and run main() in a test Two more silent passes, both the same bug as last round's, and both in what the splitter was doing with a span rather than in the span itself. arrayEntries kept only the chunks that returned to brace depth 0 and dropped the rest without a word. Measured: rewriting the switcher's Linux entry as `...LINUX_SWITCHER_ENTRIES,` left its `label` unchecked against the registry displays and its `slug` unchecked against `routeSegment`, and the gate printed OK - the input that makes useFrameworkItems build `/undefined/add-sdk`. It returns `{ entries, other }` now, and both readers report `other`. registryInvariantErrors tested the whole entry text, so a NESTED `routeSegment` satisfied the one invariant here that guards a runtime URL rather than a rendering. `{ slug: "hosted", routeSegment: null, agentSkills: true, meta: { routeSegment: "id-bolt" } }` exited 0. It reads each entry's own top level now, via the same helper introduced last round for exactly this - and so does registryValues, which counted nested occurrences by design: a nested `display: "Bogus Name"` made that name acceptable in every data file. `balanced` is quote-aware too. A closing bracket inside a string ended the span early, so `[{ slug: "a]" }, ...M]` read as ending inside the value. And the structural finding: extracting the reporting functions last round relocated the gap rather than closing it. The readers were pinned and the reporting was pinned, but nothing pinned that main() CALLS any of it - twelve separate decisions, including `declaredFrameworks` itself, could each be deleted with a green suite. Gutting the frontmatter reader printed "0 declare a framework" and OK. Two answers, because a test alone is not enough: - The counters are part of the check now. Zero pages declaring a framework in a 616-page corpus, or zero names resolved across three data files, is a broken reader rather than a clean corpus, and the script says so. - scripts/fixtures/verify-frameworks-fixture.cjs builds a small tree the whole script runs against, with ROOT overridable by VERIFY_FRAMEWORKS_ROOT. Eighteen rows, each breaking one thing and asserting the message. Eleven of the twelve mutations now fail a test; the twelfth is the dataNamesChecked guard, which is belt-and-braces by construction - any route that reaches it is already reported by dataFileErrors - and the comment says so rather than implying it carries weight. Smaller ones from the same review: - The whole-value anchor: `ios: "iOS" + SUFFIX` read as "iOS" and reported nothing, in both the object and the enum reader. Anchored and pinned. - The `myslug` fixture. The word-guard case used `mySlug`, camelCase, which a case-sensitive `slug` pattern never matches with or without the guard - it passed either way and pinned nothing. - A `/` also opens a regex after `return`, `=>`, `typeof` and friends. Without that, `return /["']/.test(s)` opened a phantom string on the quote inside the class and the whole file came back unreadable, so an ordinary regex added anywhere in SearchBar would have turned the gate red. - unionSlugs reads through stripComments, like every sibling: a commented-out member was read as a slug, and a `;` in a trailing comment truncated the union. - `enum: []` is truthy, so a vocabulary that parsed to zero slugs passed both guards and left every downstream check with nothing to compare against. - The topLevelOnly depth clamp is unreachable now that the scan is quote-aware; kept as a backstop, and the docstring says which of the two actually fixes the case rather than crediting both. - Two false comment claims: a duplicated dangling bullet in stripComments, and "three states" where it tracks four things. pagesOnly's note now says that it filters nothing when the file list already excludes `_` names - worded to hold on #429 as well, since that block is byte-identical on both branches so the two can merge in either order. Suite is 32 tests. Every fix above was verified to bite by reverting it in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): report the pairs inside an entry, and pin the reports the rows missed The same hole one brace deeper again. arrayEntries reports every depth-0 chunk of the ARRAY it cannot read, and objectLiteralValues reports every PAIR it cannot read. Reading a field off an entry did neither: entryField collected the pairs that matched and discarded the rest, so a spread INSIDE an entry was invisible in both readers of both files. const HOSTED_EXTRAS = { agentSkills: true }; { slug: "hosted", display: "Hosted", routeSegment: null, ...HOSTED_EXTRAS }, At runtime that is `agentSkills: true` with a null route - the exact input the invariant exists to catch, since resolveAgentSkillsUrl then builds /sdks/undefined/agent-skills - and the gate printed OK. `tsc --strict` accepts it, so nothing else was going to notice. Same shape in the switcher, where a later spread wins over `slug` and useFrameworkItems builds /undefined/add-sdk. entryPairs classifies an entry's own depth-0 chunks now, and both callers report what it could not read. One fixture row certified a check it never exercised. `registry-spread-entry` REPLACED an entry, which left the FrameworkSlug union listing a slug the literal no longer had - so the row passed on the union check, and the `other` reporting loop it was supposed to pin could be deleted with all rows still green. It appends the spread now, keeping the three entries, and asserts the message only that loop produces. And eight report decisions in main() were still deletable with a green suite, including the three frontmatter sentinels - which are this gate's oldest bug class - and both self-checks the previous commit added. Ten new fixture rows: an unterminated fence, `framework:` with no value, `framework: 5`, an empty docs/, an absent docs/, a renamed FRAMEWORKS, a removed FrameworkSlug union, a renamed SearchBar map, and a spread inside an entry in each of the two files. Twelve of thirteen mutations now fail a row; the thirteenth is the dataNamesChecked guard, still belt-and-braces by construction. Three smaller ones: - An absent docs/ threw a raw ENOENT stack. It is a sentence now, which matters more since ROOT became env-overridable. - VERIFY_FRAMEWORKS_ROOT is honored only alongside VERIFY_FRAMEWORKS_FIXTURE=1, and the resolved root is echoed when it is not the repo. This is a blocking gate: one stray environment value would otherwise point it at another tree and print OK for that tree, with only the `N docs scanned` line to show it. - The regex-after-keyword test excluded a property of that name. `counts.in / 2` read as the `in` keyword, opened a regex on the division, and made the whole file unreadable - fail-closed, but a false positive on valid JS, which is the shape that test was added to remove. Two comment corrections. The count assertion between entries and slugs is no longer an independent cross-check now that both come from the same reader - an entry declaring `slug` twice would balance a miss elsewhere, and it is `tsc` (TS1117) that rules that out; the comment says so. And the note on the overridable ROOT no longer implies the coverage gap is closed: the rows cover the decisions they exercise, and every round that found a deletable decision found it by mutation, so the honest instruction is to add a row when you add a check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): make the keyword guard actually work, and pin the Vale cap The guard shipped in the previous commit did nothing, and its comment said the opposite. `emit` reset `word` to "" on any non-identifier character, the dot included, so REGEX_AFTER_WORD only ever saw `in` and matched it through the `^` alternative whether or not `.` was in the character class. The documented false positive reproduced verbatim at that HEAD: stripComments("const half = counts.in / 2;") -> null stripComments("const half = counts.in / 2; // note") -> comment survives The first turns the gate red on valid JavaScript; the second is worse in kind, because a commented-out entry downstream of such a division would read as live code. One line - the dot now survives into `word` - and both behave. Three cases added to the untouched table, including the one the review named; the mutation is caught now, where reverting the character class was a no-op. Repairing the `registry-spread-entry` row last round removed the only coverage of a live check. That row used to assert `FrameworkSlug lists "X", the registry has no such entry`; making it additive moved the expectation onto the `other` message and nothing took its place, so the union loop became deletable with every test green. It matters: `tsc` accepts an extra union member, and FRAMEWORK_BY_SLUG then resolves undefined at runtime with no cast and no type error. Added `registry-extra-union-slug`. And the docs-gate half of this PR had no harness at all. The Vale frontmatter cap - the most consequential behavioural change here - could be switched off two different ways with all tests green, and both are regressions this branch already fixed once each: a metadata-only file dropped from the Vale list (0d05551ca, "the skip disabled a blocking check") and a cap that drops everything or nothing (c28f18e77, "make it fail closed"). The two decisions are pure exported functions now: - capAlerts(json, frontmatterOnly, root) - keeps frontmatter alerts, drops body alerts on files whose body is byte-identical to base. Pinned including a ceiling of 0, which is what frontmatterEndLine returns for a page with no frontmatter and must not mean "cap everything". - partitionForVale(files, bodyChanged) - every changed file reaches Vale except one whose frontmatter has no readable extent, and only the metadata-only ones carry a ceiling. Five mutations across those two now fail a test. The fixture header says plainly that the rest of docs-gate's main() is still driven by nothing, so the asymmetry does not read as a judgement. Three smaller ones: - entryField was the only reader rejecting a quoted key. `{ "slug": "ios" }` is legitimate TypeScript and objectLiteralValues already accepted it; here it reported "no plain `slug` literal" about an entry that has one. Fixed, with both quote styles pinned. While correcting it I also found the regex had come out of my own patch script with a concatenation artifact in the character class - it matched `"slug"` by accident rather than by construction. - VERIFY_FRAMEWORKS_ROOT-without-the-flag is pinned, so the guard that stops a stray environment value pointing this gate at another tree cannot be undone quietly. - Two comments corrected. declStart's said the leading word boundary fixes the `FRAMEWORKS_ORDER` prefix case; the trailing `\s*(?::|=)` is what does, and removing the leading guard leaves the behaviour correct - it stops something else, and the comment now says which. entryPairs' listed three shapes it reports and there are more, a quoted key containing a space among them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docs-gate): drive the gate itself, and stop trimming the body's leading edge Pinning capAlerts and partitionForVale as functions did not pin that main() calls them with the right arguments - and the wiring is part of the decision. Ten one-token edits at those call sites survived all 35 tests, three of them the same regression this branch has already fixed twice. Swapping partitionForVale's two arguments, which the extraction itself made possible, took the gate from exit=1 ✗ [vale:Scandit.Banned] Avoid 'Obviously' … (line 2) to `0 error(s)` and exit 0, with a real alert dropped. So docs-gate has a harness now, the counterpart to the one verify-frameworks got: scripts/fixtures/docs-gate-fixture.cjs builds a real git repository - base commit, an origin/main ref, the mutation applied as a working-tree change, and its own one-rule .vale.ini so nothing has to be fetched. Seven rows, and nine of the ten mutations above now fail one. The tenth is cspell, and the fixture header says so rather than leaving it to be rediscovered: runCspell resolves its binary under ROOT, which is the fixture and has no node_modules, so no row can tell `runCspell(files)` from `runCspell(bodyChanged)` - a distinction that matters, since a PR rewriting only `description: "Add the SDK to your Reakt Native projekt"` had its spelling check skipped entirely before 0d05551ca. Building that harness immediately reproduced the review's other finding, which it had reported as unreproducible end to end. `bodyOf(before).trim() === bodyOf(after).trim()` admitted any change that is purely LEADING whitespace, and indentation is not cosmetic in Markdown: de-indenting the first body line turns an indented code block, which Vale skips, into a paragraph, which it lints. Such a file was classified metadata-only, capped at the frontmatter, and the new alert dropped: before the fix: exit=0 docs-gate: 0 error(s) after: exit=1 ✗ Avoid 'Blatantly' … (line 6) `sameBody` trims the trailing edge only - a missing final newline is still not a body change anyone needs to review - and it is exported and pinned both ways. Four comment corrections, three of them in text the previous two commits wrote: - partitionForVale is documented as deterministic rather than pure. It reads each file through frontmatterEndLine, which is why its own test writes real files - the docstring contradicted the test beside it. - The verify-frameworks fixture header no longer claims docs-gate's decisions are covered by being extracted. - `word` is described as what it is: the identifier run ending here, prefixed by `.` when a dot precedes it, and NOT reset by whitespace. That last part is deliberate, so `return /re/` is seen, and it has a cost worth stating - a keyword separated from an identifier by whitespace alone reads as part of it. - keyRx's `\[` alternative is gone. It was dead (topLevelOnly blanks the inside of `[KEY]` but leaves the brackets), and the comment claimed a computed key was accepted. Dropping it is also the safer direction: had it matched, `[SLUG_KEY]: "ios"` would have landed in `pairs`, where entryField's anchored match rejects it - read by nothing and reported by nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix the two CI wiring defects and stop parseSdksRoute dropping the product build-docs.yml ran `yarn test:docs-gate` with no Vale on PATH. The suite asserts Vale is present rather than letting the prose rows pass vacuously, so that job went red on every PR, every push to main and the daily schedule. Moved the step into docs-gate.yml, which already installs the pinned binary; the fixture writes its own .vale.ini with no Packages line, so it needs the binary only and never the synced Google styles. The docs-gate rows ran the gate with an inherited environment, so GITHUB_BASE_REF leaked in. On a PR into release/**, which both workflows trigger on, the gate resolved origin/release/8.6 - a ref the self-contained fixture never creates - and exited 2, failing the (clean) row on a clean tree. Reproduced with CI=true GITHUB_BASE_REF=release/8.6: 1 failed, 36 passed. The child env now clears GITHUB_BASE_REF and pins CI, so an unresolvable base stays loud instead of degrading to a working-tree diff that reads as a pass. parseSdksRoute returned {} for an unregistered framework segment, dropping `product` with it. That flips isKnownProductPage to false and serves <SkillsCallout variant="shared" />, which defaults to iOS - so a new SDK tree added under docs/sdks/<new>/<known-product>/ before its registry entry landed would show an iOS-pointing Agent Skills banner on every one of its pages, silently. It now resolves the framework to undefined and keeps the product, which is what the regex it replaced did: the product callout bails on a missing framework and renders nothing. The baseline had no row for this shape; three now pin it, and they fail against the old return. tsc --noEmit clean. test:frameworks 16 passed, verify:frameworks OK, test:docs-gate 37 passed under both a bare environment and CI=true GITHUB_BASE_REF=release/8.6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop two comment blocks the refactor left duplicated Both maps carried their pre-refactor comment immediately above their replacement. The stale copies were not just redundant, they were wrong about the keying: QUERY_FRAMEWORK_TO_PATH is keyed by framework slug, not by "the ?framework= query slug", and HOMEPAGE_FRAMEWORK_ALIASES maps onto canonical slugs rather than onto QUERY_FRAMEWORK_TO_PATH's keys. Kept the concrete spellings the old block named - react, netIos, netAndroid - since which aliases exist is the one thing the derived version does not show at a glance. tsc clean, test:frameworks 16 passed, verify:frameworks OK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop two unused exports, and stop three comments describing the wrong thing FRAMEWORK_BY_SLUG and FRAMEWORK_BY_DISPLAY were exported and never imported - only named in a fixture comment. In a change whose thesis is "five copies collapsed into one", two unused derived maps are new surface that becomes a sixth. Removed. parseSdksRoute's comment claimed the omitted `framework` and `lastSegment` keys made the shape match what the previous regex produced. They do not: the old code always returned all three keys, with undefined values where nothing resolved. It is a deliberate shape change, and it matters because deepStrictEqual distinguishes an absent key from one holding undefined. Consumers read the fields rather than enumerating keys, so runtime behaviour is identical either way - the comment now says that instead of claiming a preservation that never happened. PARSE_SDKS_ROUTE_BASELINE's header said it was captured from the parser BEFORE the refactor and that it pins "the anchor that rejects /foo/sdks/...". Both were wrong. Rows copied from the old parser would fail on key presence alone, so they…
47c85c4 to
2d23e04
Compare
|
A designer needs to see ProductChooser, IntentNavigator and ErrorCodeFinder to review them - they ARE their interactions, so a screenshot shows almost nothing and a .tsx shows less. The three live on three different refs, so there was no single thing to look at. This puts all three on one route, /component-preview, so the PR preview gives one URL in real site chrome: real tokens, real fonts, real spacing, and a working light/dark toggle. It is a src/pages route, NOT a docs page, so it is in no sidebar, no framework tree and no docs version, and it is noindex. Nothing here changes the documentation; the preview is removed when this PR closes. The components are copied unmodified, via `git checkout <ref> -- <paths>` rather than cherry-pick, so none of the unrelated work on those branches (a package-lock deletion, the SkillsCallout rewrite, Vale rules, docs-gate scripts) comes along: ProductChooser docs/product-chooser (PR #417) IntentNavigator docs/phase-e-intent-navigator (+ src/data/intents.json) ErrorCodeFinder 0fd2e0a4 on docs/sparkscan-ios-rewrite (+ src/data/troubleshooting-errors.json) ProductChooser normally reads the host page's frontmatter; the page passes the same shape by hand, copied verbatim from docs/sdks/ios/sparkscan/intro.md, and renders it three times - SparkScan on iOS, the same product on Web to show the availability chips and outgoing links following the reader's framework, and MatrixScan Pick for a different block length. ErrorCodeFinder still carries its SAMPLE- placeholder entries. The page says so in a callout, so the reviewer looks at layout and interaction rather than wording. Verified: tsc --noEmit clean, and `docusaurus build` succeeds with the page rendering all three components server-side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A product-overview decision block, fed entirely from single sources of truth so the visible content can never drift from the machine-readable metadata: - "Choose this if you want to" <- the page's `user_intents` frontmatter - "Consider another product if" <- the page's `not_for` frontmatter - "Available on" matrix <- src/data/products.json (the product registry the Agent Skills pages already use), keyed by the page's `product` Demonstrated on the SparkScan iOS intro. Renders nothing if the fields are unset. Frontmatter is passed in as a prop (MDX exposes `frontMatter` in scope). Note: products.json lists Titanium for SparkScan, which contradicts the availability stubs (SparkScan is not available on Titanium) — a data inconsistency to reconcile separately (which source is authoritative). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… matrix products.json marks frameworks a product does NOT support with version "n/a" (e.g. SparkScan on Titanium). The chooser now filters those out of the "Available on" matrix, so SparkScan correctly shows 9 frameworks without Titanium. Fixes the data-vs-display mismatch; no products.json change needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nother product' item Each not_for line that names another product (e.g. '…— use MatrixScan Count') now renders that product name as a framework-aware link to its About page, resolved from products.json (SSOT): the current framework's URL, else Web, else any supported. Longest-name match so 'MatrixScan Count' wins over 'MatrixScan'. Falls back to plain text when there's no match. No frontmatter schema change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The chooser read its text from each page's frontmatter (`user_intents`, `not_for`) and looked the page's `product` up in products.json, so it found nothing on MatrixScan Batch and Smart Label Capture pages and the same text had to be copied onto every framework's page. - A product-registry plugin reads content/registry/products.yml and product-chooser.yml and passes the APPROVED chooser entries to the client as global data. It runs the verify:registry checks first, so a malformed entry fails the build with a readable message. - The component takes no props: it reads the page's framework and product from the URL (parseSdksRoute) and renders nothing without an approved entry. Each "consider another product" item links to that product's overview on the same framework and docs version (intro, else get-started; /hosted/ for hosted products), or says it is not available on this framework instead of linking to a stub. Frozen versions render nothing, because the text and availability are not versioned. - It sits once in each shared intro partial (SparkScan and the five MatrixScan products), before the first section, so every framework's intro gets it. - Column titles are styled paragraphs, not h4, so the page outline does not jump from h1 to h4; dark mode uses the lighter title colours. The SparkScan iOS intro's frontmatter fields are removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2d23e04 to
d38a07d
Compare
raffaelefarinaro
left a comment
There was a problem hiding this comment.
🤖 Automated review, posted on Raffa's behalf and not yet read by him. Treat the findings as a starting point, not a verdict. Anything that looks wrong probably is, so push back.
Start with _about-matrixscan-ar.mdx. The sentence before the new <ProductChooser /> ends with a colon: "It can be used to add the following customizabe UI elements to your workflows:". The chooser now sits directly under that colon, so once the text is approved the sentence will introduce the chooser instead of the UI elements list. The component needs to go above that sentence or after the list. Details inline. The other five partials look fine to me: the chooser sits after a complete paragraph in each.
What is good. Reading the product and framework from the URL through parseSdksRoute and frameworkToSlug removes the per-page wiring and the key mismatches. I checked that the products.json framework keys (.NET iOS, React Native, ...) are the same display names parseSdksRoute returns, and that the registry keys and products.json keys agree. Passing only status: approved entries through the plugin, and running the same chooserErrors check at build time, is a good guard: scripts/verify-registry.cjs checks every choose_if and not_for entry is a string or object of the right shape, so a malformed YAML entry fails with a readable message instead of throwing during SSR. The hosted targets resolve too: docs/hosted/express/overview.md and docs/hosted/id-bolt/overview.md exist.
Intent questions and things I could not verify.
- All five checks are green on
d38a07de(gh pr checksitself timed out on the GraphQL call, so I read the check runs through the REST commits endpoint). But there is noproduct-chooser.ymlin this PR, so nothing renders anywhere. The build exercises the plugin and the empty path only.isAppRoute(the@generated/routeslookup), the link building and the "not available on" branch have never run. Would you add a fixture entry to the test script, or do a local build with a draft file flipped toapproved, before the text PR lands? I did not run a build myself. isFrozenVersionreturns null for/7.6.15and/6.28.11. As far as I can seeversioned_docs/*/carry their own copies of the intro pages and do not import these partials, so the guard may never fire. Is it there for a case I am missing, or only as a precaution? Harmless either way.choose_ifandwhenrender as plain text, so a backticked name likeBarcodeBatchwould show the backticks. Is plain text intended for the product owners' copy?- When
not_foris empty the "Choose" column keeps half the width and the right half is blank (.decideis1fr 1fr). Is that intended?
Previous review threads on this PR. I read all three channels. There is one earlier review and seven inline threads from this account, all anchored to commits 47c85c4 and 2d23e04, which are no longer in the PR (the branch was rewritten, head is now d38a07de). Against the current head:
- Duplicated body in
docs/sdks/ios/sparkscan/intro.mdand the red Vale gate: stale. That file is no longer in the diff and the Docs Quality Gate is green. Recommend closing. productkey mismatch (matrixscan,label-capture): stale. The schema enum now uses registry keys, the threeshipping-label-scanning.mdpages are migrated, and the component resolves aliases throughparseSdksRoute. Recommend closing.user_intentsscalar crash and the "who readsuser_intents/not_for" thread: stale. The component no longer reads frontmatter. Recommend closing.FW_TO_PJduplicate mapping and theuseDoc()suggestion: stale. Both are gone. Recommend closing.- Heading levels (
h4): fixed, the titles are now<p>. The other half of that thread, the chips being spans rather than a list, is still open and is re-raised inline below. - The
FeatureListreuse note from the review body (show the version) is still open, also inline below.
I have not replied in or resolved any thread.
- Move the chooser above the sentence that introduces the MatrixScan AR UI elements, so the colon stays attached to its list; fix "customizable". - Column titles use the body text colour; the coloured dot carries the cue. The "Available on" label moves to a darker shade for contrast. - "Available on" is a list, with each framework's version. - Backticked names in the chooser text render as inline code. - One full-width column when a product has no alternatives. - Explain when the frozen-version guard fires. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Build alternative links from the framework's route segment, so .NET pages (/sdks/net/ios/) link instead of falling back to plain text. - Also try a product's single page, for flat products like Barcode Generator. - Chips read "iOS since 6.8"; Linux says "since C API 4.6". - Detect frozen snapshots from the active docs version instead of the URL, so it works under a base URL. - Share the inline-code renderer with FeatureList. - Darker amber dot for graphic contrast; list item spacing; one title rule. - MatrixScan AR intro names the right product, and the chooser follows the "powered by" sentence like the other MatrixScan intros. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Type the product-registry plugin as a Docusaurus Plugin, so `npm run typecheck` passes again. - Render only on the in-development docs: products.json and the chooser text describe docs/, not the stable snapshot served during a beta. - Resolve alternative links from the active version's doc paths instead of walking the generated route tree. - One products.json "supported" check and lookup map, shared with FeatureList; one route parse; Linux's C API numbering lives in the framework registry. - Index keys for chooser items; list spacing that doesn't depend on CSS load order; the MatrixScan AR sentence after the box names its subject. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The six KMP intro pages carry their own text instead of the shared partials, so they get <ProductChooser /> directly, before their first heading. - Look up alternative links only when an entry renders, instead of indexing every doc path on each intro page. - Drop FeatureList's unused products.json and Product imports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
raffaelefarinaro
left a comment
There was a problem hiding this comment.
🤖 Automated review, posted on Raffa's behalf and not yet read by him. Treat the findings as a starting point, not a verdict. Anything that looks wrong probably is, so push back.
Start with CI: the last four commits have never been built or gated. gh pr checks 417 returns nothing because the base is now feat/product-registry, and build-docs.yml, docs-gate.yml and docs-preview.yml only run for PRs into main or release/**. The last green runs are on d38a07de. These landed after it without a run: 333d9c7f (placement, contrast, list semantics), e500b5e1 (.NET links, frozen-version detection), abfd0ccd (plugin typecheck, current-docs-only) and d7599164 (the six Kotlin Multiplatform intros, which the docs gate lints as whole pages). I think a retarget fires edited, which these workflows do not listen to, so the retarget will probably not start a run either. Cheapest fix: once #487 merges and the base is main, push an empty commit and wait for Build Documentation and the Docs Quality Gate before merging. Also, yarn typecheck exists in package.json but no workflow runs it, so the typecheck commit is only checked on the author's machine.
What looks good: the product and framework now come from the URL through parseSdksRoute, so there is nothing for a page to forget or mistype. The component returns null for anything not approved, on any docs version other than current, and for a framework with no route. isSupportedVersion is shared with FeatureList, so the two cannot disagree about n/a.
What I checked at head d7599164:
node scripts/verify-registry.cjspasses (17 products, 683 docs) andnode scripts/test-registry.cjspasses (67 tests). I ran these with js-yaml 4 from a scratch directory, not the lockfile's^5.2.2, and without a full install, so treat it as a smoke test.- Both ends of the single source of truth agree: the
docs-schema.ymlproduct enum equals the registry keys, andURL_PRODUCT_MAPPINGmapsmatrixscanandlabel-capturetomatrixscan-batchandsmart-label-capture. - The comment that no snapshot imports these partials holds: versioned pages import
versioned_docs/version-X/partials/..., notdocs/partials. - Every
/hosted/<product>/overviewpath the component builds exists for Express and ID Bolt. FeatureListbehaves the same: no empty orN/Aversions exist infeatures.jsonorproducts.json.
Not verified: yarn build and yarn typecheck (no dependencies installed here), how the block renders, and Vale on the six KMP intro pages.
Open question on the chooser text file: content/registry/product-chooser.yml has no documented shape. AGENTS.md says the text goes there, but nothing says the fields are status, choose_if and not_for: [{when, use}], nor that the headings are "Choose X if you want to" and "Consider another product if", so each choose_if line must continue "you want to" and each when is shown as "{when}: {Product}". Product owners are about to write this file. Could a commented example entry go next to products.yml, or into the AGENTS.md Products section? (Those files are in #487, so I cannot anchor it inline here.)
Smaller points are inline. One copy note: the _about-matrixscan-ar.mdx edit is reader-visible today ("MatrixScan is powered" becomes "MatrixScan AR is powered", "customizabe" is fixed, one sentence is split). Both are improvements, but the description says readers see no change yet.
Existing threads, checked against head. These are all from earlier automated reviews under this account, and I did not reply to or resolve any:
3759466848(SparkScan iOS block copied from the partial): fixed.docs/sdks/ios/sparkscan/intro.mdis no longer changed. Recommend closing.3759466865(product key lookup misses MatrixScan Batch and Smart Label Capture),3759466875(frontmatter casts),3759466885(GEO consumer ofuser_intents),3759466892(framework map and iOS fallback link),3759466902(prop versususeDoc): all fixed or moot sinced38a07de. Product and framework come from the URL, the frontmatter fields are gone, and a product missing on the reader's framework renders as text. Recommend closing.3759466910(h4 headings, span chips) and4267324802(title contrast): fixed in333d9c7f(paragraph titles,ul/lichips, body-colour titles).4267324812(chip list and version): fixed in333d9c7f. Recommend closing.4267324794(colon sentence in_about-matrixscan-ar.mdx): fixed in333d9c7f. The chooser now sits after the "powered by" sentence. Still shows as unresolved because the anchor line did not move. Recommend closing.4267324818(nothing exercises link building): still true in substance, see the inline comment onoverviewUrl.
- Link building moves to ProductChooser/links.ts, and scripts/test-product-chooser.cjs pins it: base URL and preview prefix, intro, get-started and flat-page fallbacks, hosted overviews, and the .NET and Kotlin Multiplatform route segments. CI runs it, plus `yarn typecheck`, which no workflow ran. - "Consider another product if" lists only products available on the reader's framework; with none left, the block shows one column. - An empty or comments-only product-chooser.yml no longer fails the build. The plugin comment says why a draft with a bad `use` still does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…lter - The plugin reads product-chooser.yml with the verifier's readChooser, so an empty or comments-only file builds on js-yaml 5 too. - The "Consider another product if" filter moves to links.ts as visibleAlternatives, with tests. The test loader resolves the loaded file's imports from its own folder. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks. Addressed in 183c36a here and in 0f6af2c on #487. CI: you're right, nothing has run here since d38a07d. Locally at head:
Build Documentation now also runs Chooser file shape: #487 now ships
AGENTS.md points to the file. Copy note: agreed, the MatrixScan AR edit is visible today. The description now says so. The earlier threads you listed as fixed are resolved. The remaining ones have replies inline. |
raffaelefarinaro
left a comment
There was a problem hiding this comment.
🤖 Automated review, posted on Raffa's behalf and not yet read by him. Treat the findings as a starting point, not a verdict. Anything that looks wrong probably is, so push back.
Start with the merge path, not the code. The description says "Merge #487 first; GitHub then retargets this PR to main". GitHub only retargets when the base branch is deleted, and this repo has delete_branch_on_merge set to false (checked with gh api repos/Scandit/data-capture-documentation). So unless "Delete branch" is clicked when #487 merges, the PR will stay on feat/product-registry and still get no CI. That matters because build-docs.yml, the gate and the preview only run for PRs into main or release/**, and nothing has run here since d38a07de. After the retarget, as you planned, an empty commit is needed, because edited does not start these workflows. I found no code defect that blocks this PR.
What looks good. The product and framework come from the URL through parseSdksRoute, so a page has nothing to forget or mistype. visibleAlternatives and overviewUrl are plain functions with type-only imports, which is what lets test-product-chooser.cjs run them under Node. The new readChooser is shared by the plugin and verify:registry, so the build and CI read the file the same way.
What I ran, since CI has not. I extracted head 183c36a8 into a scratch directory (not the worktree), installed from the lockfile, and ran:
yarn typecheck: clean. This is the new CI step, so it should not fail every PR after merge.test:product-chooser(14 passed),test:registry(72 passed),verify:registry(17 products, 683 docs): all pass.readChooserpluschooserErrorson an empty file, a comments-only file, a BOM-prefixed file and a bareproducts:: no errors. A draft with a typo inusestill errors, as you intend.- A preview-mode build (
preview_build=true, base URL/data-capture-documentation/pr-preview/pr-417/) with a temporary approved entry forsparkscanandmatrixscan-batch. The build succeeded. I read the built HTML: on iOS, KMP and .NET iOS SparkScan intros the two "consider" rows link to<base>/sdks/<route>/matrixscan-count/intro/and.../matrixscan-ar/intro/. On the Web SparkScan intro the MatrixScan Count row is hidden and only AR remains. A product with nonot_forrenders one column. The SparkScan intro has a singleui-overviewanchor. The broken-link warnings in that build are all/7.6.15/.../release-notes/links from the frozen versions that preview builds drop, not from this PR.
Not verified: Vale on the six KMP intros, and how the block looks in a browser (contrast, dark mode, mobile).
Existing threads, checked against head 183c36a8. All are from earlier automated reviews under this account. I did not reply to or resolve any. These five are still open and are fixed, so I recommend closing them:
4267324818(nothing exercises link building): fixed in124fa24f, which addsscripts/test-product-chooser.cjsand runs it in CI. It also now passes in my scratch run.4279649985(empty or comments-onlyproduct-chooser.ymlfails the build): fixed in183c36a8, confirmed above.4279649991(nothing runsoverviewUrl, preview base URL): fixed in124fa24f, with tests at the site root and under the preview prefix. My preview build agrees.4279649994(unavailable alternative shown as a dead end): fixed in124fa24fwithvisibleAlternatives. Confirmed on the Web build.4279649999(KMP intros are copies of the partials): answered. The author's correction shows they are KMP-specific rewrites, so the tag in two places is intended.
The other ten threads are already resolved.
Two small, non-blocking points are in my notes rather than here. Question about intent: since the chooser renders only on the current docs version, product owners' approved text will not show at the site root until the next release makes current the root version. The description says this, so I read it as intended.
|
Thanks for running the checks CI couldn't.
Still open: how the block looks in a browser (contrast, dark mode, mobile). It can't render until an approved entry lands, so I'll check it on the preview of that PR. |
Stacked on #487 (product registry). Merge that first. This repo keeps merged branches, so GitHub won't retarget this PR by itself: after #487 merges, change the base to
main(or click Delete branch on #487), then push an empty commit. Build Documentation, the Docs Quality Gate and the preview only run on PRs intomain, and editing the base doesn't start them. Until then, no CI runs here.Summary
A "Choose this if / Consider another product if" block, plus the frameworks the product is available on, for product overview pages.
How it works now
content/registry/product-chooser.yml, keyed by registry product keys. A newproduct-registryplugin passes onlystatus: approvedentries to the page, so drafts never render. It runs theverify:registrychecks first, so a malformed entry fails the build with a readable message.<ProductChooser />reads the page's framework and product from the URL (parseSdksRoute, aliases resolved). That fixes the earlier lookup that found nothing on MatrixScan Batch and Smart Label Capture pages./hosted/for hosted products). An item whose product isn't available on the reader's framework is not shown; with none left, the block shows one column.scripts/test-product-chooser.cjspins the links and the filter, and CI runs it.src/data/products.json.current) render it: the text and availability aren't versioned.h4, so the page outline doesn't jump from h1 to h4; dark mode uses lighter title colours.The chooser itself isn't visible yet.
content/registry/product-chooser.yml(from #487) has no entries. The text is with product owners; approved entries land in their own PR.One reader-visible change: the MatrixScan AR intro partial now says "MatrixScan AR is powered…" instead of "MatrixScan is powered…", fixes "customizabe", and splits one sentence so the chooser can sit after it.
CI: Build Documentation now also runs
yarn typecheckandyarn test:product-chooser.