Repository navigation
feat: define the shared WinUI sample index contract (#703 Phase 0) - #806
Merged
Merged
Conversation
Jaylyn Barbee (Jaylyn-Barbee)
added
the
corpus-rebake-not-needed
Copilot started reviewing on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 14:03
View session
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The schema currently validates samples that the parser drops or interprets as C#, weakening the core published contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Defines the Phase 0 shared WinUI sample-index contract for #703 without changing shipped CLI behavior.
Changes:
- Adds the schema plus shared parser and deterministic writer.
- Migrates Reactor parsing to the shared implementation.
- Adds opt-in index generation and comprehensive contract tests.
File summaries
| File | Description |
|---|---|
docs/winui-sample-index.schema.json |
Defines the published index contract. |
SampleIndexSchema.cs |
Centralizes contract fields and version. |
SampleIndexParser.cs |
Parses indexes into provider data. |
SampleIndexWriter.cs |
Generates deterministic indexes. |
ReactorFetcher.cs |
Delegates parsing to the shared parser. |
SnapshotBaker.cs |
Adds per-provider index generation. |
Program.cs |
Adds the build-only --emit-index option. |
SampleIndexTests.cs |
Tests parsing, round trips, inheritance, and schema alignment. |
WinApp.Cli.Tests.csproj |
Copies the schema into test output. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Build Metrics ReportBinary Sizes
Test Results✅ 4786 passed, 5 skipped out of 4791 tests in 740.3s (+22 tests, -118.6s vs. baseline) Test Coverage✅ 89% line coverage, 82.3% branch coverage · CLI Startup Time53ms median (x64, Try This BuildInstalls the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing. & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 806Switching between builds often?Put the tool on your PATH once: & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPathThen this build is just: winapp-pr 806Run Updated 2026-09-04 13:55:07 UTC · commit |
Copilot started reviewing on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 14:30
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Generated Gallery indexes bypass sanitization and contain malformed XAML that violates the contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced
Copilot started reviewing on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 14:59
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Malformed language values are accepted as C#, and index generation can produce incomplete or mixed-generation artifacts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/winapp-CLI/WinApp.Cli.SnapshotBaker/SnapshotBaker.cs:229
- What is wrong: each provider's index is published directly to the final directory before the run is known to be complete. Show me: run successfully once, then rerun when one provider fails: successful providers are replaced while the failed provider's old
index-*.jsonremains, so the directory still looks complete but mixes generations even though the command exits 1. Why it matters: a stale or internally inconsistent artifact set can be handed to upstream maintainers. Smallest fix: stage the whole index set and publish it only whenfailuresis empty, asBakeAsyncalready does.
src/winapp-CLI/WinApp.Cli/Services/Controls/SampleIndexParser.cs:119 - What is wrong:
GetStringmakes an absentlanguageand a present value of the wrong JSON type indistinguishable. Show me:{ "code": "auto x = 1;", "language": 42 }produces a scenario whose code is treated as C#, although the contract says only an absent tag or the stringcsharpis accepted. Why it matters: malformed network content can be presented as paste-ready C# instead of being rejected. Smallest fix: distinguish a missing property from a non-string property and accept the code only for missing or case-insensitivecsharp; cover the non-string case in the parser tests.
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
Copilot started reviewing on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 16:00
View session
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The parser rejects the schema-valid schemaVersion representation 1.0, silently producing an empty corpus.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/winapp-CLI/WinApp.Cli/Services/Controls/SampleIndexParser.cs:176
schemaVersion: 1.0is valid under JSON Schema 2020-12 for bothtype: "integer"andconst: 1, butJsonElement.TryGetInt32rejects that lexical representation. A schema-valid index produced by a serializer that writes1.0therefore parses as an empty corpus instead of v1. Compare the numeric value (for example viaTryGetDecimal) and add this representation to the version-gate test.
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We are the consumer, so we define the format. Before asking WinUI-Gallery or CommunityToolkit to publish a sample index, this establishes the contract and proves it can carry real sample data losslessly -- so each upstream PR arrives as "here is the schema and the tested code that produces it" rather than a request for someone else to design one. The schema generalizes the index microsoft/microsoft-ui-reactor already publishes, keeping its field names verbatim so that file stays valid without the Reactor team changing anything. Additions cover XAML sources: `xaml`, `xmlnsImports`, `docs[]`, and `details` for long-form prose (Reactor's `description` already means the one-line summary, and Gallery keeps both). `additionalProperties` stays open because the live Reactor index carries `generatedFrom`, `category` and `galleryRoute` that this contract does not model. ReactorFetcher is rewired onto the shared reader rather than left duplicating it, dropping it from 151 to 67 lines. That is what makes this evidence instead of speculation: the reader now runs against a real published index in production. Behavior is unchanged, so CacheVersion is not bumped. Two hardening wins fell out of the shared reader: non-object entries inside `controls`/`samples` are skipped instead of throwing, and `Scenario.Source` is stamped by the caller rather than read from the document, so a source cannot label its samples as another source's. SampleIndexWriter has no non-test caller yet by design -- it is the Phase 1 deliverable, and gains one when the reference generator is wired up (deferred behind #770, which owns the corpus bake it will build on). Verification: 193/193 targeted find-ui, controls, search and sample-index tests pass; Release build of the full solution is clean with 0 warnings. Beyond the 12 hermetic tests, a throwaway run against the live Reactor corpus confirmed all 93 real scenarios survive write/parse field-for-field with byte-identical output across runs, and `find-ui "flex layout" --source reactor` still returns Flex and StackPanel end to end. Refs #703 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…se 0) Validating the Phase 0 contract against all three live corpora surfaced two places where the schema could not represent the data losslessly. Both would have shipped as silent quality regressions once a source published an index. Curated keywords are a separate ranking signal ---------------------------------------------- ProviderData carries two dictionaries, not one: SearchEngine weights Keywords at 5.0 and Tags at 3.0. Only the Toolkit populates Keywords, from author-written markdown frontmatter. The contract had a single `keywords` field, so a Toolkit index would have folded its best signal down to 3.0 and quietly re-ranked results. `keywords` keeps its published meaning (Tags, 3.0) because Reactor already ships that field and re-pointing it would change live rankings. The new `curatedKeywords` carries the 5.0 slot. Verified against the live Toolkit corpus: 26 of 26 controls populate both. Details and imports belong to the sample, not only the control -------------------------------------------------------------- The writer took control metadata from the first scenario in each group. For `description`, `apiNamespace` and friends that is correct - they are control facts. For `details` and `xmlnsImports` it is not: each Toolkit sample is its own markdown file with its own XAML, so siblings legitimately disagree. `segmented` has five samples with five descriptions and differing imports (2/2/2/2/3). Collapsing them onto the first would have emitted XAML using a prefix the index never declared. Both fields are now valid at either level. The writer hoists them to the control when every sample agrees and writes them per sample when they do not, so the common case (Gallery, where all samples share one description) stays compact: the gallery index grows 412 bytes, the toolkit index 5.7 KB, reactor not at all. The reader prefers the sample value and falls back to the control's. Proving it against the real corpora ----------------------------------- A throwaway harness fetched all three sources live and round-tripped every scenario through writer and reader. Two findings worth recording: - `gallery/itemsrepeater` has one scenario scraped without any control metadata. Round-tripping fills it from its siblings. That is a repair, not a loss, so the permanent contract is "a populated value is never changed or dropped" rather than strict equality. - `gallery-tags.json` holds 13 entries for control ids that have no scenarios. SearchEngine derives its control list from scenarios, so they are already dead today; the index cannot carry them and does not need to. Sanitizer behaviour is identical before and after the round trip - the same sample ids are dropped either way, so the index changes nothing the corpus boundary guard cares about. The harness itself is deleted; its durable conclusions are three hermetic tests covering per-sample divergence, control-level hoisting, and reader inheritance. Also adds `--emit-index` to the snapshot baker so the reference generator can be run on demand. Nothing is committed, following the precedent set when #770 merged with only compressed blobs checked in. The existing positional bake invocation used by build-cli.ps1 and the drift workflow is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 406ab204-0471-4344-95a7-a91ae84d5a45
The schema promised things it did not enforce, so an upstream publisher
could pass validation and still silently lose samples:
- A sample's anyOf only checked that xaml or code was present, but the
parser drops both on IsNullOrWhiteSpace. `{"code": " "}` validated
and then vanished. Require a non-whitespace character instead.
- `language` accepted any string while the parser always treats code as
C#, so a sample tagged cpp would be pasted into a user's .cs file.
Constrain it to csharp, and drop non-C# code in the parser so a
publisher who skips validation still cannot inject another language.
- The xaml description told authors to declare prefixes on the control,
which contradicts the sample-level xmlnsImports override our own
Toolkit index relies on.
Verified all 466 samples across the gallery, toolkit and reactor
indexes still validate, and generator output is byte-identical.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 406ab204-0471-4344-95a7-a91ae84d5a45
Three fixes to the generated index and the reader. --emit-index wrote raw provider output. ScenarioSanitizer normally runs later, in ControlsSearchService, so the reference artifact we intend to hand upstream carried malformed XAML and unbalanced C# that every consumer then drops -- the exact breakage a published index exists to remove. Sanitize before serializing and skip samples left with neither XAML nor C#: 18 unusable gallery samples now drop, and no malformed XAML remains in any of the three indexes. The control-level usings block is copied onto each of a control's samples, so an oversized one multiplies past the capped fetch size. Real usings are a few namespace names, so drop the prefix above a generous ceiling. Link the schema from llms.txt beside cli-schema.json, so the upstream teams meant to implement the contract can find it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 406ab204-0471-4344-95a7-a91ae84d5a45
Sanitizing before serialization means the generator can write fewer samples than it fetched, which reads as a contradiction next to the Phase 0 claim that the schema carries the corpus losslessly. Both are true, but they are different claims: losslessness is a property of the schema -- a scenario that survives is carried field for field, which is what the round-trip tests assert -- not a promise that every fetched scenario appears. Drop that distinction into the remarks so the next reader does not have to derive it from the counts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 406ab204-0471-4344-95a7-a91ae84d5a45
Jaylyn Barbee (Jaylyn-Barbee)
force-pushed
the
jay/find-ui-indexes
branch
from
September 2, 2026 19:53
1254e51 to
6cd9c76
Compare
Copilot started reviewing on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 20:09
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Index publication must be atomic, and parsing must accept every schema-valid representation of version 1.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Balanced
Copilot started work on behalf of
Jaylyn Barbee (Jaylyn-Barbee)
September 2, 2026 20:25
View session
Co-authored-by: Jaylyn-Barbee <51131738+Jaylyn-Barbee@users.noreply.github.com>
Zach Teutsch (zateutsch)
left a comment
Zach Teutsch (zateutsch)
left a comment
Contributor
There was a problem hiding this comment.
🤖 AI-generated review (winappcli pr-review skill) — verify before acting.
Decision
Changes required — the generated Gallery index includes a winapp-authored sample and presents it as upstream Gallery content. The parser’s handling of malformed language values is also worth tightening, but it is non-blocking.
Must fix
Generated Gallery indexes misattribute locally authored samples
- What is wrong: Index generation writes the post-processed Gallery corpus, including scenarios injected locally by winapp rather than supplied by WinUI Gallery.
- Show me: Running
WinApp.Cli.SnapshotBaker.exe --emit-index <dir>producedindex-gallery.jsoncontainingPhoto gallery: image grid (UniformGridLayout), which is added byGalleryFetcher.InjectMissing; expected: an upstream index containing only Gallery-owned samples. - Why it matters: The artifact intended for WinUI Gallery maintainers would ask them to publish code their repository does not own, making the proposed authoritative index inaccurate.
- Smallest fix: Mark locally injected scenarios as synthetic and exclude them in
EmitIndexesAsync, with a regression test proving emitted source indexes contain fetched source content only. - Location:
src/winapp-CLI/WinApp.Cli.SnapshotBaker/SnapshotBaker.cs:211
Non-blocking
Malformed language values are accepted as C#
- What is wrong: A missing
languageand a present non-string value both become an empty string, which the parser treats as C#. - Show me:
{"code":"auto x = 1;","language":42}flows throughGetStringand is retained as paste-ready C#; expected: reject the code because a present language must be the stringcsharp. - Why it matters: Malformed upstream data can surface a wrong-language snippet that fails when pasted into a C# project. Current published Reactor data uses valid
csharpvalues, so this is not blocking. - Smallest fix: Distinguish an absent property from a present non-string property and add that malformed case to parser tests.
- Location:
src/winapp-CLI/WinApp.Cli/Services/Controls/SampleIndexParser.cs:121
What was exercised
dotnet build src\winapp-CLI\WinApp.Cli.SnapshotBaker\WinApp.Cli.SnapshotBaker.csproj -c Debug— succeeded with 0 warnings and 0 errors.- Filtered
SampleIndexTests— all 21 passed. - Built
WinApp.Cli.SnapshotBaker.exe --emit-index <scratch-dir>— fetched all three providers and reproduced the Gallery provenance defect in generated output. scripts\build-cli.ps1— compilation and generation progressed into the full test suite, then repeatedly failed downloadingMicrosoft.Windows.SDK.BuildToolsand stopped producing output; the stalled run was stopped. Internal-feed authentication was unavailable in this shell, so the full suite did not complete.
… language tags winapp reads published sample indexes at runtime but never writes one, so SampleIndexWriter moves into the build-only SnapshotBaker alongside --emit-index. The shipped CLI keeps only SampleIndexSchema and SampleIndexParser, which ReactorFetcher already uses. Moving the file out of Services/Controls also takes it out of that subtree's relaxed .editorconfig, so it now meets the repo's brace and file-header rules. SampleIndexParser no longer treats a malformed language tag as C#. GetString collapses an absent property and a present non-string property into the same empty value, so a sample carrying `"language": 42` was retained as paste-ready C#. The tag is now read directly, and code is kept only when the tag is absent or names csharp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30e85b00-6b59-4df1-b2d7-5642c86c823a
Merged
3 of 7 tasks
Contributor
DecisionMerge. This is a well-scoped Phase 0 for #703: a published sample-index schema, a shipped reader that Reactor already uses, and a build-only writer + Must fixNone — mergeable as-is. Non-blockingBaker treats an unknown flag as the output directory
What was exercised
|
Zach Teutsch (zateutsch)
approved these changes
Sep 8, 2026
Zach Teutsch (zateutsch)
left a comment
Zach Teutsch (zateutsch)
left a comment
Contributor
There was a problem hiding this comment.
Approved with one non blocking comment.
Jaylyn Barbee (Jaylyn-Barbee)
added a commit
that referenced
this pull request
Sep 11, 2026
…es them (#815) ## Description `GalleryFetcher` post-processed the corpus it fetched from WinUI-Gallery, then stamped the results `Source = "gallery"`. That is the value `find-ui` renders as the `[gallery]` tag and uses to build ids like `gallery-itemsrepeater-7` — so **winapp presented its own content to users as WinUI Gallery's.** Three mechanisms, found by decompressing the baked corpus rather than by reading the code: | Mechanism | What it did | State when found | |---|---|---| | `InjectMissing` — CommandBar | Appended a hand-written CommandBar sample | **Dead.** Guarded on Gallery having no commandbar sample; Gallery ships one now. | | `InjectMissing` — `itemsrepeater-7` | Appended a hand-written `UniformGridLayout` photo grid | **Live, unconditional.** No detection logic at all. | | `ApplyOverrides` — `tabview-1` | Rewrote upstream's `CreateNewTab` C# body | **Live.** Guarded on `CSharp.Contains("Frame")`. | Both live paths are removed and the dead one is deleted outright. `FetchAsync` is now a passthrough. This is wrong on its own terms, and it blocks [#703](#703): we cannot ask WinUI-Gallery to publish a sample index while quietly merging samples they do not own into their data — still less hand them a *generator* that does it. A mislabeled local file is a bug; a generator that silently injects a third party's content into their published output is something else. **The guidance is kept and stays reachable.** It moves to a surface that says who wrote it: - The image-grid pattern moves to `Notes.cs`, so it appears in the **Important** notes on every `ItemsRepeater` result instead of masquerading as an upstream sample. - The TabView advice was **already** in `Notes.cs` (*"TabViewItem.Content can be ANY UIElement... use Frame only for page navigation"*), so dropping that override loses nothing. It was redundant when it was written. This also follows the repo convention of stating each user-facing fact once on its canonical surface. ## Usage Example No CLI surface changes. The observable difference is that a sample winapp wrote no longer appears under the `[gallery]` tag: ```console $ winapp find-ui "photo grid" ``` Before, this returned `gallery-itemsrepeater-7` — a winapp-authored sample labeled as Gallery's. Now it returns `GridView`, `ItemsView` and `ItemsRepeater`, and the `UniformGridLayout` guidance reaches the user through the **Important** notes on the `ItemsRepeater` result, attributed to winapp. ## Related Issue Unblocks [#703](#703). Companion to #806, which defines the index contract; this PR makes the data we would generate from that contract honest. ## Type of Change - 🐛 Bug fix ## Checklist - [x] New tests added for new functionality — `GalleryCorpus_CarriesNoWinappAuthoredSample` - [x] Tested locally on Windows - [ ] Main README.md updated — n/a - [ ] docs/usage.md updated — n/a, no command surface changed - [ ] Language-specific guides updated — n/a - [ ] Sample projects updated — n/a - [x] Shipped skills updated in `plugins/winapp/skills/` — `winapp-find-ui/SKILL.md` ## Additional Notes ###⚠️ This PR *does* re-bake the corpus — do not apply `corpus-rebake-not-needed` Unlike #806, this one genuinely changes the baked blobs. `snapshot-gallery.json.br`, `snapshot-reactor.json.br` and `snapshot-manifest.json` are all updated. It also bumps **`CacheVersion` 19 → 20**, so the manifest is re-stamped and every existing on-disk cache is discarded. See below for why that is load-bearing rather than housekeeping. ### Isolating the change from upstream drift The committed corpus was stale, so a naive before/after would have conflated our removal with upstream's own additions. Both were baked back to back against the same live upstream: ``` main 323 scenarios branch 322 scenarios ``` The only two differences: - `itemsrepeater-7` — removed. - `tabview-1` — C# restored to upstream's (939 → 1064 chars), confirming the baked corpus really had been carrying winapp's `CreateNewTab` body. The CommandBar injection produced no difference, which is the direct confirmation it had gone dead. Committed counts move **321 → 322** rather than 323 → 322 because upstream added two samples since the last bake. Manifest now reads gallery 322 / reactor 95 / toolkit 48. ### The one real gap this leaves The `itemsrepeater-7` injection was not baseless — `UniformGridLayout` inside `ItemsRepeater` is genuinely under-demonstrated upstream. Verified against the corpus: four upstream samples use it, each demonstrating something else. That gap is now addressed as **guidance in `Notes.cs`** rather than as a fabricated sample. The TabView override was a different matter: upstream ships ten `tabview` samples and we rewrote one of them to taste. That is not a gap. ### Regression guard `GalleryCorpus_CarriesNoWinappAuthoredSample` fails if a winapp-authored sample reappears in the Gallery corpus, so this cannot silently regress. ### Review feedback **The stale cache was the real bug.** Removing the injection and re-baking does not reach anyone who already has a cache. Caches are matched on an exact version string, so a user who fetched with the shipped CLI keeps a `19` cache holding the winapp-authored sample and the rewritten `tabview-1`, and keeps being served them under the `[gallery]` tag indefinitely — including offline. `CacheVersion.cs` states the rule directly: bump whenever *"cleaning logic changes would alter the cached output for the same input data."* That is exactly this change, so `Current` is now `20`. Worth noting the re-bake at version 20 produced **byte-identical Brotli blobs** — the only diff is the manifest stamp. The corpus content was already right after the first bake; what was missing was the instruction to discard old caches. **The provenance claim was overstated, in code and in the skill.** Both said samples are reproduced "as fetched" / are "exactly what that repository ships". That is not true: `FetchFromGitHub` cleans content, compresses C#, truncates both languages at 2000 chars, and strips event handlers with no emitted code-behind. The skill compounded it by telling users to report anything wrong to WinUI-Gallery, so the wording could have sent people upstream to file defects winapp's own extraction introduced. Both now state the narrower invariant this PR actually establishes — no injected scenarios, no bespoke per-sample overrides — and say plainly that extraction is uniform and deliberately lossy. **Moving the note to `Notes.cs` quietly cost discoverability.** `Notes.Get` is consulted only when rendering a selected scenario (`SearchEngine.cs:825`); notes are never part of the BM25 document. The removed sample's header was the only text in the corpus carrying the token `photo`, so deleting it made the guidance reachable only to someone who already knew to look at `ItemsRepeater`: ``` before this PR photo grid -> ItemsRepeater (the winapp-authored sample) after removal photo grid -> Grid, Image <- guidance unreachable after the fix photo grid -> GridView, ItemsView, ItemsRepeater ``` Fixed in `Synonyms`, which is query-time only and needs no re-bake, so the ranking change carries no corpus risk. `thumbnail` already mapped to `Image` and now also offers the controls that lay thumbnails out. **Not taken:** extending the regression guard to fingerprint the old `tabview-1` body. The guard still covers only the injected sample, so re-introducing that specific override would not fail the build. ### Verification - **171** targeted tests pass (`Gallery`, `FindUi`, `Controls`, `Search`, `EmbeddedSnapshot`, `Notes`, `Synonyms`, `CacheVersion`, `Toolkit`, `Reactor`). - Release build clean — **0 warnings**, warnings-as-errors on. - `scripts/validate-plugin-package.ps1` clean. - Live CLI run confirms the `ItemsRepeater` note renders in the **Important** section. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Zach Teutsch <88554871+zateutsch@users.noreply.github.com> Copilot-Session: 30e85b00-6b59-4df1-b2d7-5642c86c823a
This was referenced Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
#703 asks WinUI-Gallery and CommunityToolkit to publish machine-readable sample indexes so
find-uistops scraping their repository layouts. Neither team has ever been asked for this — research found zero prior art in either repo — so appetite is genuinely unknown, and that is the main risk on the issue.This is Phase 0, which is entirely on our side of the fence. We are the consumer, so we define the contract first. The point is that each upstream ask arrives as "here is the schema, here is the file it produces, here is the tested code that produced it" rather than asking two maintainer teams to design a format for us.
Nothing here changes user-facing behavior.
What ships:
docs/winui-sample-index.schema.jsonschemaVersionpinned toconst: 1.SampleIndexParserScenario[]. Kind-checks every element, since this parses untrusted network content.SampleIndexWriterReactorFetcher--emit-indexThe vocabulary is deliberately Reactor's.
microsoft-ui-reactoralready publishesreactor-search-index.json, and this contract generalizes it rather than competing with it. Field names are the ones Reactor already ships, which means Reactor's live index validates against this schema today with no change on their side — that is the proof the contract is adoptable rather than aspirational, andReactorFetcherreading it through the shared parser is the working demonstration.Usage Example
Not a shipped CLI surface. The generator is opt-in on the build-only baker:
# Generate an index per source into a scratch directory (never committed) dotnet run --project src/winapp-CLI/WinApp.Cli.SnapshotBaker -- --emit-index ./artifacts/indexesVerified live against all three sources. This is a usable-only export: scenarios are sanitized first and any left with neither XAML nor C# are omitted, so gallery writes 305 of the 323 it fetches. Those 18 are already dropped before search today, so including them would hand upstream known-broken content to publish. "Lossless" here is a property of the schema — a scenario that survives is carried field for field — not a promise that every fetched scenario appears.
Related Issue
Closes #808 — Phase 0 of #703.
Type of Change
Checklist
SampleIndexTests.csplugins/winapp/skills/— n/a, no CLI workflow changedAdditional Notes
This PR needs the
corpus-rebake-not-neededlabel, and here is the evidencefind-ui-corpus-check.ymlfires on two changed paths —WinApp.Cli.SnapshotBaker/andReactorFetcher.cs— and this PR re-bakes nothing. That is correct behavior from the check and the right question to ask, so it was answered directly rather than waved off:Rewiring
ReactorFetcheronto the shared parser (−97/+20 lines) is behavior-preserving. Gallery and Toolkit are the control: their fetchers are untouched and also came out identical, which confirms both bakes saw the same upstream state, so a reactor difference would have been attributable to this code rather than to upstream moving mid-test.Separately and unrelated to this PR: the committed corpus is now behind upstream (gallery 321 → 323, reactor 93 → 95, baked 2026-08-11). That is exactly what
find-ui-corpus-drift.ymlexists to report, and it is deliberately not folded in here.Two schema defects that only showed up against real data
Both were found by round-tripping all three live corpora, and both would have shipped as silent quality regressions once a source published an index.
1. Curated keywords are a separate ranking signal.
ProviderDatacarries two dictionaries, not one:SearchEngineweightsKeywordsat 5.0 andTagsat 3.0. Only the Toolkit populatesKeywords, from author-written markdown frontmatter. A singlekeywordsfield would have folded the Toolkit's best signal down to 3.0 and quietly re-ranked results.keywordskeeps its published meaning (Tags, 3.0) because Reactor already ships that field and re-pointing it would change live rankings for no reason. The newcuratedKeywordscarries the 5.0 slot. Against the live Toolkit corpus, 26 of 26 controls populate both — so this is load-bearing, not hypothetical.2.
detailsandxmlnsImportsbelong to the sample, not only the control. The writer took control metadata from the first scenario in each group. Fordescription,apiNamespaceand friends that is correct — they are control facts. For these two it is not: each Toolkit sample is its own markdown file with its own XAML, so siblings legitimately disagree.segmentedhas five samples with five descriptions and differing import counts (2/2/2/2/3). Collapsing them onto the first would have emitted XAML using a prefix the index never declared.Both fields are now valid at either level. The writer hoists them to the control when every sample agrees and writes them per sample when they do not, so the common case stays compact — Gallery, where all samples share one description, grows 412 bytes; Toolkit grows 5.7 KB; Reactor not at all. The reader prefers the sample value and falls back to the control's.
Two findings about existing data, recorded rather than fixed
gallery/itemsrepeaterhas one scenario scraped with no control metadata at all —ApiNamespace,ControlDescription,Description,RelatedControlsandDocsare all empty onitemsrepeater-7while its six siblings have them. Round-tripping fills it from the siblings. That is a repair, not a loss, so the contract asserted in tests is "a populated value is never changed or dropped" rather than strict equality. Fixing the scrape is out of scope here and moot once Gallery publishes an index.gallery-tags.jsonholds 13 entries for control ids that have no scenarios (scratchpad,listbox,wrappanel,jumplist-gallery, …).SearchEnginederives its control list from scenarios, so these are already dead today. The index cannot carry them and does not need to.Design calls worth reviewing
additionalProperties: truethroughout. Reactor's live index carriesgeneratedFrom,categoryandgalleryRoute, which this contract does not model. A closed schema would reject the one real-world index that already exists, which would be an odd contract to hand someone.detailsis a new field rather than a redefinition ofdescription. Reactor's publisheddescriptionalready means the one-line summary. Gallery keeps both a short subtitle and a long-form description, so it needs a second field — takingdescriptionfor the long form would have broken the one index in the wild.schemaVersionis accepted as v1; an unknown one is refused. A future v2 index should fail loudly rather than be half-parsed into wrong results.generatedAtUtcis omitted by default, andJavaScriptEncoder.UnsafeRelaxedJsonEscapingkeeps XAML<from becoming\u003C. A generated artifact that churns between runs cannot be diffed by a drift check and is unreviewable by the maintainers we are asking to host it.build-cli.ps1and the drift workflow is deliberately byte-identical.Review feedback
The writer moved out of the shipped CLI.
--emit-indexwas always a baker flag, butSampleIndexWriteritself sat inWinApp.Cli/Services/Controls/, so the shipped binary carried code whose only caller is a build-time tool. It now lives inWinApp.Cli.SnapshotBaker/. The CLI keeps onlySampleIndexSchemaandSampleIndexParser— the parts that survive into a world where upstream publishes and we only ever read. Moving it also took the file out of theServices/Controls/.editorconfigsubtree, so it now meets the repo's brace and file-header rules.A malformed
languagetag no longer yields C#.GetStringcollapsed absent and present-but-not-a-string into"", so"language": 42parsed as C# and would have been offered as paste-ready code.IsCSharpnow inspects theJsonElementdirectly and treats a non-string tag as not-C#. Covered for42,nulland"".Zach's blocking finding is fixed in #815, not here. He flagged that
--emit-indexemits winapp-authored samples presented as Gallery content, and proposed filtering them at emit time. Investigating it showed the divergence was not an emit-time problem:GalleryFetcherwas injecting and rewriting samples in the fetch path and stamping themSource = "gallery", so they were already reaching users mislabeled, long before any index was written. Filtering at emit would have hidden the symptom on the one surface we were about to show upstream. #815 removes the divergence at the source instead, which also makes the filter unnecessary. This PR is therefore unchanged in that respect and its byte-identical-corpus evidence below still stands.Verification
SampleIndex,ReactorFetcher,ScenarioSanitizer,Controls,FindUi,Gallery,Toolkit,Search,EmbeddedSnapshot).Schema_DeclaresExactlyTheFieldsTheReaderAndWriterUseassertsdocs/winui-sample-index.schema.jsonandSampleIndexSchemacannot drift apart — the schema file is copied to the test output for that check.