Repository navigation
docs(knowledge): every component spec lives in docs/specs/components, one folder per component - #9007
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC |
|---|---|---|
| BottomSheet | 34.2 kB (0) · 7.4 kB CSS (0) | — |
| BottomSheetSwitcher | 14.9 kB (0) · 3.4 kB CSS (0) | — |
| Center · Storybook | 4.2 kB (0) · 4.7 kB CSS (0) | 118 (0) |
31 more components
| Component | Size | LoC |
|---|---|---|
| ChatComposer | 64.0 kB (0) · 11.1 kB CSS (0) | — |
| ChatComposerDrawer | 24.5 kB (0) · 4.7 kB CSS (0) | — |
| ChatComposerInput | 58.7 kB (0) · 10.3 kB CSS (0) | — |
| ChatComposerTokenElement | 58.7 kB (0) · 10.3 kB CSS (0) | — |
| ChatDictationButton | 49.3 kB (0) · 9.8 kB CSS (0) | — |
| ChatLayout | 51.9 kB (0) · 10.2 kB CSS (0) | — |
| ChatLayoutScrollButton | 49.3 kB (0) · 9.9 kB CSS (0) | — |
| ChatMessage | 23.6 kB (0) · 3.7 kB CSS (0) | — |
| ChatMessageBubble | 7.1 kB (0) · 3.7 kB CSS (0) | — |
| ChatMessageList | 29.8 kB (0) · 7.9 kB CSS (0) | — |
| ChatMessageMetadata | 26.5 kB (0) · 4.1 kB CSS (0) | — |
| ChatSendButton | 48.6 kB (0) · 9.4 kB CSS (0) | — |
| ChatSystemMessage | 6.8 kB (0) · 3.8 kB CSS (0) | — |
| ChatTokenizedText | 7.1 kB (0) · 4.0 kB CSS (0) | — |
| ChatToolCalls | 32.8 kB (0) · 9.2 kB CSS (0) | — |
| Code · Storybook | 3.6 kB (0) · 3.5 kB CSS (0) | 81 (0) |
| Drawer | 33.4 kB (0) · 8.2 kB CSS (0) | — |
| DrawerHeader | 51.8 kB (0) · 11.5 kB CSS (0) | — |
| Markdown | 148.4 kB (0) · 18.2 kB CSS (0) | — |
| Pagination | 88.6 kB (0) · 15.7 kB CSS (0) | — |
| PowerSearch | 173.5 kB (0) · 20.3 kB CSS (0) | — |
| Selector | 80.1 kB (0) · 15.4 kB CSS (0) | — |
| SelectorOption | 16.0 kB (0) · 5.4 kB CSS (0) | — |
| Timer · Storybook | 9.5 kB (0) · 6.9 kB CSS (0) | 186 (0) |
| Tokenizer · Storybook | 73.1 kB (0) · 16.9 kB CSS (0) | 1,963 (0) |
| lab/CommentComposer | 97.8 kB (0) · 1.9 kB CSS (0) | — |
| lab/CommentPin · Storybook | 50.2 kB (0) · 1.2 kB CSS (0) | 413 (0) |
| lab/Comments | 225.5 kB (0) · 2.5 kB CSS (0) | — |
| lab/CommentThread | 220.9 kB (0) · 2.4 kB CSS (0) | — |
| lab/EmojiPicker | 66.3 kB (0) · 722 B CSS (0) | — |
| richtext/RichTextEditorToolbar | 216.6 kB (0) · 454 B CSS (0) | — |
Package size: @astryxdesign/core 5.64 MB packed (−950.5 kB); no new dependencies.
API changes: core +2 · ~1 · −0 (report stale: run pnpm api:report)
3 API changes (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | core |
. · Types · interface SearchContext |
interface SearchContext<T extends SearchableItem = SearchableItem> |
import type {SearchContext} from '@astryxdesign/core' |
| + | core |
./Typeahead · Also exported from an earlier entry point · SearchContext |
SearchContext (.) |
import {SearchContext} from '@astryxdesign/core/Typeahead' |
| ~ | core |
. · Types · interface SearchSource |
search: (query: string) => Promise<T[]> | T[] → search: (query: string, context?: SearchContext<T>) => Promise<T[]> | T[] |
import type {SearchSource} from '@astryxdesign/core'; const x: SearchSource = {search: …} |
Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
CI impact: 0 job-min per update · pool 0% · time to green +1.1 min · standard runners · 13 CI action or script files changed (not measured)
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 78.6 | 78.6 | 0 |
| Runner pool, runner-minutes a day | 36,679 | 36,679 | 0 (0%) |
| Average concurrent jobs | 25.5 | 25.5 | 0 |
| Time to green (median → this run) | 15.5 min | 16.6 min | +1.1 min |
| Runs a day | 466 | 466 | 0 |
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/a11y-pr-contracts.test.mjs, .github/scripts/analyze-pr.test.mjs, .github/scripts/change-scope.cjs, .github/scripts/change-scope.test.mjs, .github/scripts/ci-interaction-webkit.test.mjs, .github/scripts/component-audit-scope.test.mjs, .github/scripts/component-design-decisions.test.mjs, .github/scripts/knowledge-paths.cjs, .github/scripts/knowledge-paths.test.mjs, .github/scripts/lib/classify-visual.test.mjs, and 3 more.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Generated by PR Enrichment workflow | View full report
|
/approve-spec a7ec724 |
|
Posted by Cindy's agent on her behalf: the approval above applies Cindy's approval of this spec move (component and module records under docs/specs/components/, knowledge-contracts placement decision, AST-093 FR1/FR6). |
|
/approve-spec a4007e9 |
a4007e9 to
ca7b641
Compare
|
/approve-spec ca7b641 |
There was a problem hiding this comment.
The record move itself checks out: lookups from component source files return the same records as before, every moved record resolves to its owning package, and the knowledge and gate suites pass. Two changes are needed before this lands:
- Keep
packages/<pkg>/src/**/<Name>.spec.mdclassified as a knowledge record inchange-scope.cjs(and flip the new expectation atchange-scope.test.mjs:183-190), because otherwise the spec-owner gate reports "No knowledge records changed." for the 121+ open pull requests that still edit records at those paths, while merging them carries the edits intodocs/specs/components/. - Reword "in the subdirectory their code has" in
docs/architecture/knowledge-contracts.mdanddocs/README.md, or move the records, since 12 of the 14 module records (for exampleMarkdown/modules/softBreaks.spec.md, whose code is inMarkdown/plugins/) are not there andcheck:knowledgedoes not check it.
packages/native/apple/README.md also still lists the moved Swift specs under Sources/Astryx/.
[Automated review]
| {filename: 'packages/core/src/Selector/Selector.spec.md'}, | ||
| ]); | ||
| expect(result.specOnly).toBe(false); | ||
| expect(result.touchesKnowledgeRecords).toBe(false); |
There was a problem hiding this comment.
Once this is on main, spec-owner-gate.yml runs this classifier from the default branch, so touchesKnowledgeRecords: false here means an open pull request that still edits packages/core/src/<X>/<X>.spec.md gets spec-owner-approval success ("No knowledge records changed.") while git's rename detection lands the edit in docs/specs/components/<X>/ on merge. At least 121 open pull requests edit records at their old paths. Classifying non-ignored packages/<registered package>/src/**/<Name>.spec.md as knowledge records (not spec-only) keeps the gate failing closed until those branches rebase; this assertion would then expect true.
| public name normally matches the root; a parent/member exception requires an | ||
| exact top-level or full inline consumer-doc entry in that root. Public | ||
| semantic module records are nested at least one directory beneath the same | ||
| folder as `<PublicName>.spec.md`, in the subdirectory their code has; the |
There was a problem hiding this comment.
12 of the 14 module records this pull request moves are not in the subdirectory their code has: for example Markdown/modules/softBreaks.spec.md governs packages/core/src/Markdown/plugins/softBreaks.ts, and DropdownMenu/modules/useMenuPress.spec.md governs packages/core/src/hooks/useMenuPress.ts. check:knowledge accepts a module record in any subdirectory, so this clause (and docs/README.md:43) is neither true today nor enforced. Rewording it, for example "mirroring the code's subdirectory where the code has one under the component folder", or moving the records and adding the check would resolve it.
…h lookup gets its own test budget (#9073) ## Maintainer impact In full node-project runs on a busy machine, `.github/scripts/a11y-pr-contracts.test.mjs` › changedSinceMergeBase › "lists the branch's own changes and never makes a full clone shallow" times out at 30 s. Alone it takes under a second. The case ran about 35 git processes in one budget. It built an origin and a full clone (about 14 git calls, plus the clone's and the push's own processes), then made four lookups of about 5 calls each, one of them a fetch that fails. On a loaded runner, starting a process is what costs. ## Intended invariant Unchanged: against a local or a remote base, the branch's own changes are listed; a complete clone is never made shallow; a base that can't be found, or no base, gives no list. ## Change This is test only: - The two repositories are built once, in `beforeAll`. - Each lookup is its own case: a local base; the remote base, with the clone still not shallow; a missing or absent base. The repositories, the lookups and the assertions are the same. No budget changes. **Tradeoff:** building the repositories is now the largest step, under the hook's own 30 s budget; the cases depend on that shared fixture (they don't change it). **Rejected:** a larger budget only moves the threshold. ## Evidence A repeatable load: the test pinned to one CPU (`taskset`) shared with 40 busy loops. - **Before** (main 45b112c): the case times out at 30 s (it needed 38 s). - **After:** the three cases take 4, 5 and 3 s. - Unloaded, the whole file passes (46 tests). - **Product:** no source file changes. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context. ## Testing 1. `pnpm exec vitest run --project node .github/scripts/a11y-pr-contracts.test.mjs` 2. The load: `for i in $(seq 40); do taskset -c N node -e 'for(;;){}' & done`, then step 1 under `taskset -c N` with `-t changedSinceMergeBase`, on main and on this branch. Prior art: #9007 edits other cases in this file (spec-record paths), with no overlap; no other open pull request touches it (searched the changed files of all open pull requests). Local gates at 1fdb412: ESLint, Prettier and `pnpm check:repo` are clean; the file passes (46 tests); the full node project passes apart from two lock tests in `cache.test.mjs`, which fail on main on this machine too (#8520 fixes them).
ca7b641 to
dc1278d
Compare
|
/approve-spec dc1278d |
78c704c to
553dbf9
Compare
There was a problem hiding this comment.
Changes requested: the record move and its tooling check out, and the new shared-root fallback fixes Core and Lab under Chat, but a Charts module record under Chart still has no placement that passes check:knowledge. knowledge-paths.cjs:222-224 looks for the module's code only under src/<Root>/, so a Charts hook such as useChartColors (exported from Charts' src/, with a Lab copy in src/Chart/) resolves to Lab or to no package. Please search a flat package's own src/ and break a tie by the package that owns the root's component (Chart.doc.mjs), with a test for a Charts module under Chart, or narrow the Owning code, docs/README.md and the DEC-15 row to exclude that case.
[Automated review]
| ), | ||
| ) ?? | ||
| pick(pkg => | ||
| hasStemBelow(path.join(root, pkg.src, componentRoot), publicName), |
There was a problem hiding this comment.
This only searches src/<Root>/, so a flat package's code is never found. Under Chart, Charts keeps useChartColors.ts and getChartColors.ts directly in src/, and Lab keeps same-stem copies in src/Chart/. A Chart/modules/useChartColors.spec.md record therefore resolves to Lab, and check:knowledge reports that the module and its parent component:Chart (Charts) sit in different component roots. A Charts-only hook resolves to no package. Searching a flat package's own src/, and breaking a tie by the package that owns Chart.doc.mjs, would match the Owning code's "the one holding the record's code".
553dbf9 to
5353227
Compare
|
/approve-spec 5353227 |
There was a problem hiding this comment.
Changes requested. The record move and its tooling check out: every tracked non-record file routes to the same records as before, a spec left at an old path stays owner-gated, the spec text is approved at this head, and CI is green. One fix from the last review remains. Under the Chart root that Lab and Charts share, resolveComponentPackage (.github/scripts/knowledge-paths.cjs:215-224) looks for a module's code only in src/Chart/. So a Charts module record such as docs/specs/components/Chart/modules/markColor.spec.md resolves to no package (and useChartColors resolves to Lab), and check:knowledge rejects every placement. Resolve a module record under a shared root to its parent component record's package, or search a flat package's src/ for the stem, and add a Charts-beside-Lab fixture to knowledge-paths.test.mjs.
[Automated review]
7483fd1 to
9ead7ac
Compare
|
/approve-spec 9ead7ac |
There was a problem hiding this comment.
This looks ready to merge: the component-spec move and its tooling check out on this head, the rewritten placement records carry an exact-head spec approval, and the Charts shared-root placement issue from the last review is fixed. One optional cleanup: the description still says 193 records (Core 179) move and that lookups return the same records for every tracked file, but the rebase also moved InfoTip's record, so 194 move and the three Lab InfoTip files now route to component:InfoTip as well. The red CI results are unrelated to this change: test-node (4/4) fails in a theme build --watch timing test (this pull request touches no CLI or theme code), and the spec owner gate run hit a GitHub API rate limit, so re-running both should clear them.
[Automated review]
Generated by a one-off move: git mv of every component record (core, lab, charts) and every Swift component spec, then path rewrites.
check:knowledge and specs:for read component and module records from docs/specs/components/<ComponentRoot>/, resolve the owning package from the source tree (by exact case, and by the records own code when two packages share a root), and compare placement against the source directory a record mirrors. A spec file left in a component packages src fails with the path it belongs at. Platform records (<Name>.swift.spec.md) sit beside the core record and stay review-checked until the schema carries platform_of.
…ontracts DEC-15, AST-093 FR1/FR6)
9ead7ac to
12ed676
Compare
|
/approve-spec 12ed676 |
1 similar comment
|
/approve-spec 12ed676 |
There was a problem hiding this comment.
This looks ready to merge: the component-spec move and its tooling check out on this head, and the rewritten placement records carry an engineering-owner /approve-spec for this exact commit. One optional fix: the description says specs:for returns the same records for every tracked file, but the three Lab InfoTip files now also reach component:InfoTip and Package.swift loses its AST-093 FR1 route, so that line could say so.
[Automated review]
User impact
Anyone working on a component, on any platform, finds all of its specs in one folder.
docs/specs/components/Avatar/now holdsAvatar.spec.mdbesideAvatar.swift.spec.md; before this change the core record sat inpackages/core/src/Avatar/and the Swift record inpackages/native/apple/Sources/Astryx/Avatar/.Change
git mv). All 194 component and module records go todocs/specs/components/<ComponentRoot>/, structured like the source tree: core 180, Lab 2, Charts 6, and Swift 6 (as<Name>.swift.spec.md). Record ids and text do not change. Module records keep their subdirectories, for exampleTable/plugins/rowStatus/useTableRowStatus.spec.md..github/scripts/knowledge-paths.cjs,scripts/check-knowledge.mjs,scripts/specs-for-file.mjs):check:knowledgeandspecs:forread records fromdocs/specs/components/.Chat/), the record belongs to the package that holds its code.specs:forreturns the same records for every tracked file.*.spec.mdleft in a component package'ssrc/fails, and the error names the path it belongs at.architecture:knowledge-contractsOwning code describes the layout, and DEC-15 records the decision.spec:AST-093FR1 places a native spec beside the core record, and FR6 countsdocs/specs/components/**/<Name>.<language>.spec.mdas native-only.architecture:component-theming-surfacegets one wording fix.Package.swiftloses its specexclude:list. Docs, AGENTS.md, native READMEs, and source comments point to the new paths.[docs]patch Changeset: component specs are no longer shipped inside the npm package; they live in the repo atdocs/specs/components/.astryx swizzleno longer copies the component's spec file into your project. Nothing a consumer imports or renders changes.Not in this change
platform_of.packages/native/specs/.Testing
pnpm check:knowledgepasses, andpnpm check:repopasses.specs:for --unroutedreturns the same paths before and after; the only addition is this pull request's own Changeset.--unreachedreturns 0. Sampled files (component, member, module, plugin module, Lab, Charts, shared Core/Lab and Lab/Charts roots) route to the same records, one for one.Writing rubric results
Posted by Cindy's agent on her behalf.