fix(build): derive Astryx surface inventory from @astryxdesign/core by liuxiaocs7 · Pull Request #3883 · apache/maka

liuxiaocs7 · 2026-08-26T07:47:30Z

Summary

The file-level Astryx surface inventory (docs/astryx-surface-file-inventory.md, generated by scripts/generate-astryx-surface-inventory.mjs and enforced by scripts/check-astryx-surface-inventory.mjs) could not see most of the design system, for two reasons — both in that one file:

The known-component set was a hand-maintained 47-name list while @astryxdesign/core ships ~194 components, so Timestamp, ChatMessage, Thumbnail, and others were invisible (0 rows) even though packages/ui/src/chat-turn.tsx renders them. Removing such a component left a row byte-identical, so the gate saw nothing (same root cause as Astryx surface inventory check is one-directional and not wired into CI #3064).
Severity only recognized raw button/input/select, so a hand-rolled twin of a shipped component scored aligned.

This PR:

Derives the Astryx component set from the shipped .d.ts (dist/index.d.ts + each export * from './Dir'). The derivation is fail-closed: an unresolved barrel target, a version that disagrees with the pinned dependency, or a parse below a floor / missing a key component aborts the run — never a silent partial set. The generated doc header records @astryxdesign/core@<version> and the export count.
Derives the @maka/ui → Astryx re-export map from the barrel export graph (packages/ui/src/index.ts, resolving export * / aliased re-export chains) instead of a hand-maintained list, so the "Astryx used" column is a true record.
Adds a conservative reimplementation tier: a public @maka/ui export whose name shadows a shipped Astryx component but is defined locally (not a re-export). Neutral wording — a review signal, not a claim of semantic re-implementation. It flags 0 files on the current tree (no false positives).
Fixes CI: the npm ci install step now runs on astryx_surface, so an inventory-doc-only PR installs dependencies before the generator runs; guarded by a new scripts/ci-test-plan.test.mjs assertion.

Fixes #3868

Verification

Ran locally (Node 24, @astryxdesign/core@0.5.0):

npm run astryx:surface-inventory:write → astryx @0.5.0: blocker=0 reimplementation=0 polish=1 aligned=213
npm run astryx:surface-inventory (coverage gate) → coverage: ok (214 files, 1 exclusions)
npm run astryx:surface-inventory:test → 9 pass (derivation version/floor/key-components; barrel re-export + nested export * + local-shadow detection; fail-closed on unresolved target; severity cases)
node --test scripts/ci-test-plan.test.mjs → all pass (incl. the new install-dependencies-covers-astryx_surface assertion)
biome check on the changed scripts + package.json, and node scripts/asf-license-headers.mjs check → clean

Not run: full npm run build / repo-wide typecheck. The change is confined to build-tooling scripts and their generated docs (the .mjs scripts are outside the workspace tsc graph).

Review response (thanks @codex)

P1 — CI installs deps for inventory-only PRs: npm ci now gates on astryx_surface; regression test added in ci-test-plan.test.mjs.
P1 — reimplementation false positives: redesigned from name-matching to public-export shadowing with neutral wording. The prior two hits were not real: markdown.tsx's Markdown is not publicly exported, and module-hub-selector.tsx's Selector is private (the public Selector is a direct Astryx re-export). Now 0 findings, no false positives.
P2 — @maka/ui re-exports auto-derived: the hand-written list is removed; the map is resolved from the barrel export graph (35 names vs the old 9).
P2 — fail-closed declaration parsing: version-vs-pin check, minimum-count and key-component assertions, and a hard error on any unresolved barrel target. (The TS compiler API was not adopted: this repo's typescript@7 is the Go-native port — platform binaries, no classic ts.createSourceFile JS API — so a hardened lightweight parser with explicit fail-closed checks is used instead.)
P3 — version recorded: the loader returns { version, components } and the doc header states the version + export count.

AI use

No generative tool made a substantive contribution
Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Claude Opus 4.8) — analysis, design of the derivation/detection, the script and test changes, and repository verification. The commit carries a Generated-by trailer.

Checklist

Tests cover the change and fail without it
Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

Yes — described under Summary above
No

Astro-Han

I reviewed this head and found no blocking issues.

Derives Astryx surface inventory from installed package .d.ts (fail-closed on unparsable, no silent fallback), with correct wrapper/re-implementation classification; hosted test/package/windows_recovery/audit/owner all SUCCESS.

No P0-P3.

简体中文

该头无阻断。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Astro-Han

I reviewed this head and found no blocking issues.

Drift from 5439e35 to 47a2dc2 is pure rebase (includes #3280); inventory derivation logic unchanged. Hosted test: FAILURE on e2e/workhub-layout is unrelated flake (Desktop E2E, not this PR's scope).

No P0-P3.

简体中文

该头为纯 rebase，无新增阻断。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Astro-Han

What this PR does

The generated docs/astryx-surface-file-inventory.md could not see most of the design system, for two reasons living in scripts/generate-astryx-surface-inventory.mjs: the known-component set was a hand-maintained 47-name list while @astryxdesign/core ships ~190 components (so Timestamp, ChatMessage, Thumbnail and friends appeared zero times across 213 rows even where they are imported and rendered), and severity only recognized raw button/input/select. This derives the component set by parsing the installed declarations — dist/index.d.ts plus each export * from './Dir' — and adds a reimplementation tier for a local component whose name matches a shipped Astryx twin it does not import.

Verdict

Comment review, not a block: 2 × P1, 2 × P2, 1 × P3, inline.

The diagnosis is right and the derivation half is well built — I checked the parser against the real 0.5.0 declarations and could not find a name it should have caught and missed (194 derived; export type blocks skipped, inline type modifiers handled, export interface XProps not mistaken for a component, SCREAMING constants and useX hooks correctly excluded). Output is byte-stable and locale-independent, and the version is exact-pinned in both manifests and the lockfile.

The two P1s are about the other half. One is mechanical: the gate became install-dependent without moving into the install condition, so an inventory-only PR now red-checks with "run npm install". The other is that the new reimplementation tier, on the current tree, produces two false positives and zero true positives — and one of them is exactly the "wrapper that renders the twin" shape the PR says it excludes.

The thread running through it

The PR names its own root cause well: "a generated artifact whose input is hand-maintained drifts in whichever half the author forgets." It then derives one of the two hand-maintained inputs and leaves the other. MAKA_UI_ASTRYX_REEXPORTS (:146) still hand-lists 9 names while packages/ui/src/index.ts re-exports 34, and the same loop at :281-291 now reads the derived set for one decision and the hand set for the other. So the drift is smaller but structurally identical, and 26 files still under-report.

The second thread is that "wrapper that renders the twin" was implemented as a same-file static-import check, which does not match how this repo actually writes wrappers — markdown.tsx reaches its twin through React.lazy across a file boundary. Widening the exclusion to follow a local re-export/lazy hop, or narrowing the tier to names that are unambiguous rather than generic, would both work; shipping it as-is puts wrong advice into an artifact CI enforces.

Mechanics

The conflict with main is only on the generated file (main deleted runtime-host-project-directory-editor.tsx; 214 rows there against 213 here). Rebase plus npm run astryx:surface-inventory:write resolves it — no hand-editing.

This does not make #2645 materially worse: it conflicts on the same file, its resolution is the same single regenerate command before and after, and it touches package-lock.json, so code=true and the P1 install bug does not hit it. It does mean #2645 can no longer resolve by hand-editing rows.

Note (ungraded)

The churn is understated by omission — the body lists the new names and tiers but not that 66 of 213 rows changed content. The stated totals (blocker 0, reimplementation 2, polish 1, aligned 210) reproduce exactly.

AI use: Claude Code assisted with running the generator, comparing the regenerated artifact, and reading the installed @astryxdesign/core declarations; both P1s were re-verified against the exact head by me, and the analysis, grading, and conclusions are my own. The worktree was restored after regeneration.

简体中文

这个 PR 在做什么：生成的 docs/astryx-surface-file-inventory.md 看不见设计系统的大部分，原因有两个都在生成脚本里——已知组件集是手工维护的 47 个名字，而 @astryxdesign/core 实际有约 190 个（于是 Timestamp、ChatMessage、Thumbnail 这些在 213 行里出现 0 次，哪怕文件真的 import 并渲染了它们）；严重度又只认原生 button/input/select。本 PR 改为解析安装版声明文件（dist/index.d.ts 加每个 export * from './Dir'）来推导组件集，并新增 reimplementation 档：本地组件名撞上已发布的 Astryx 同名件、且没有 import 它。

结论：comment review，不阻塞，2 × P1 / 2 × P2 / 1 × P3，全部在行内。

诊断是对的，推导那一半也做得扎实——我拿真实的 0.5.0 声明文件核了解析器，找不到应该收进来却漏掉的名字（推导出 194 个；export type 块正确跳过，行内 type 修饰符正确处理，export interface XProps 没被误判成组件，全大写常量和 useX hook 也都正确排除）。输出字节稳定、与 locale 无关，依赖在两个 manifest 和 lockfile 里都精确锁版。

两条 P1 都在另一半。一条是机械问题：这个门槛变成依赖安装了，却没有把 astryx_surface 移进 install 的条件，于是只改生成文件的 PR 现在会红，还配一句误导的 "run npm install"。另一条是新的 reimplementation 档在当前代码树上是 2 个误报、0 个真阳性——而其中一个恰恰就是 PR 自称会排除的"包了一层但渲染的还是同名件"。

贯穿其中的线索：PR 自己把根因写得很准——"输入靠手工维护的生成物，总会在作者忘记的那一半漂移"。但它只推导了两个手工输入中的一个。MAKA_UI_ASTRYX_REEXPORTS（:146）仍然手写 9 个名字，而 packages/ui/src/index.ts 实际再导出 34 个；:281-291 同一个循环里，一个判断读推导集、另一个读手工集。漂移变小了，结构完全一样，26 个文件仍在少报。第二条线索是"包一层就排除"被实现成了同文件静态 import 检查，而这个仓库真正的包装写法不是那样——markdown.tsx 是经 React.lazy 跨文件到达同名件的。

机械层面：与 main 的冲突只在生成文件上，rebase 后跑一次 npm run astryx:surface-inventory:write 即可，不需要手改。对 #2645 没有实质加重：同一个文件冲突，解法前后都是同一条重新生成命令，且它动了 package-lock.json，code=true，不会踩到上面那条 install 的坑；唯一影响是 #2645 不能再靠手改行来解冲突了。

不计分：改动量在描述里被省略了——正文列了新名字和新档位，但没提 213 行里有 66 行内容发生变化。给出的总计（blocker 0, reimplementation 2, polish 1, aligned 210）能精确复现。

Astro-Han · 2026-08-26T18:13:18Z

        if: steps.plan.outputs.code == 'true' || steps.plan.outputs.astryx_surface == 'true'
-        run: npm run astryx:surface-inventory
+        run: |
+          npm run astryx:surface-inventory


P1 — this gate is now install-dependent, but its step is not gated on npm ci. Reachability ①.

Install dependencies (.github/workflows/ci.yml:127-128) runs on code || asf_source || cli_package || release_contract. This step runs on code || astryx_surface. astryx_surface && !code is reachable, and it is exactly the case a regeneration produces: isAstryxSurfaceInventoryPath matches docs/astryx-surface-file-inventory.md, while isDocumentation treats everything under docs/ as documentation, so code stays false.

Verified in the worktree:

planTests(['docs/astryx-surface-file-inventory.md']) → { astryxSurface: true, code: false, ... }

On main the generator needed no dependencies — the component set was a literal. This PR makes it call require.resolve('@astryxdesign/core'), so with no node_modules it now exits 1 with cannot derive Astryx component set: @astryxdesign/core did not resolve — run npm install. Any PR whose only change is the generated inventory — including a rebase that just regenerates it — red-checks with that misleading message.

The comment at ci.yml:65-67 explicitly classifies this family of regenerate-and-diff gates as running "on Node alone… rather than behind an install", and setup-node's own condition (:91-92) already lists astryx_surface separately from install. Adding astryx_surface to the install condition is the smallest fix. The new npm run astryx:surface-inventory:test in this same step needs it too.

Astro-Han · 2026-08-26T18:13:18Z

+ * A component definition: `function Name` / `class Name`, or a `const`/`let`
+ * bound to an arrow, `function`, `forwardRef`, or `memo`. The arrow/function
+ * requirement is deliberate — it excludes the dynamic-component idiom
+ * `const Icon = table[x]` (a render-time binding, not a re-implementation).


P1 — the reimplementation tier's entire current yield is false positives. Reachability ①.

I regenerated the inventory (output byte-identical to the committed artifact) and read both flagged files. Neither is a reimplementation:

packages/ui/src/markdown.tsx — the local Markdown is a React.lazy wrapper:

const MarkdownBody = lazy(() => import('./markdown-body.js').then((m) => ({ default: m.MarkdownBody })));

and packages/ui/src/markdown-body.tsx:34 does import { Markdown as AstryxMarkdown } from '@astryxdesign/core/Markdown' and renders it at :152. This is precisely the "wrapper that renders the twin" case the PR says it excludes — the exclusion is implemented as a check over static import specifiers in the same file, so a one-hop lazy boundary defeats it. The advice the row emits ("use the shipped twin") is wrong, and markdown-body.tsx sits two rows away marked aligned — uses Astryx (CodeBlock, Link, Markdown).

packages/ui/src/module-hub-selector.tsx:47-64 — the local Selector renders <TabList>/<Tab> from @astryxdesign/core (imported at :21). Astryx Selector is a dropdown; this is a tab strip. Pure name collision.

Across all 213 surface files only 4 define a component whose name is in the derived 194-name set (AppShell, CommandPalette, Markdown, Selector), and the two not flagged escape only via a direct twin import. So the tier scores 0 true positives and 2 false positives today, in an artifact CI enforces as truth.

The collision surface also grew sharply with derivation: the derived set now contains Icon, Item, Text, Link, Code, Card, Section, Grid, Center, Step, Token, Field, Toast, Overlay, Outline, Theme. Any private helper named one of those is flagged as a design-system violation.

generate-astryx-surface-inventory.test.mjs:71-79 locks the markdown.tsx shape in as "the canonical #3868 case", so this is a deliberate choice rather than an oversight — but it is wrong on the real file, and I do not think the tier is ready to ship in this shape.

Astro-Han · 2026-08-26T18:13:18Z

        localToCanonical.set(local, original);
-        used.add(original); // imported = available; count as used if also JSX or always?
-        // Import alone is enough to claim the primitive is in the dependency set;
-        // prefer also requiring JSX below for "uses".
      } else if (isMakaUiRoot && MAKA_UI_ASTRYX_REEXPORTS.has(original)) {


P2 — the hand-maintained list survives at :146, and this loop now reads two different sets for one fact.

The PR replaces the 47-name Astryx list with derivation, but this sibling 9-name hand list still governs the @maka/ui root-barrel path. packages/ui/src/index.ts re-exports 34 Astryx names straight from @astryxdesign/core — I extracted them:

Badge Banner Button Card CheckboxInput CheckboxList CheckboxListItem ClickableCard CommandPalette CommandPaletteFooter CommandPaletteInput Divider EmptyState FormLayout HStack IconButton InputGroup InputGroupText Kbd MoreMenu NumberInput RadioList RadioListItem Selector SelectorOption Spinner Stack StackItem StatusDot Switch Text TextArea TextInput VStack

Nine of those are in the list. So a file importing TextInput, Switch, FormLayout, MoreMenu, StatusDot or TextArea from @maka/ui still under-reports its "Astryx used" column — e.g. apps/desktop/src/renderer/settings/memory-settings-page.tsx and settings/password-input.tsx. Removing one of those components from such a file still leaves its row byte-identical, which is the exact failure mode #3868 describes.

What makes this structural rather than incidental: inside the same loop at :281-291, localToCanonical is populated from this hand list while twinBindings is populated from the derived set. One fact, two representations, already disagreeing. The PR body says it derives "instead of maintaining a second list" — the second list is still here.

Astro-Han · 2026-08-26T18:13:18Z

+  collectDeclaredComponents(indexSrc, set);
+  for (const m of indexSrc.matchAll(/export \* from ['"]\.\/([^'"]+)['"]/g)) {
+    const dir = m[1];
+    const dirIndex = join(distDir, dir, 'index.d.ts');


P2 — resolution can yield a silently partial component set, which contradicts the stated invariant.

The PR says resolution failure is a hard error and never a silent fallback. That is true for the two failures the code names above (package unresolvable, index.d.ts unreadable) and false for three partial-parse paths:

if (existsSync(dirIndex)) … else if (existsSync(flat)) … has no else. A top-level export * from './Foo' whose declarations move (.d.mts, dist/types/Foo/, …) is skipped without a word.

The regex only matches ./-relative star exports at the top level; a nested export * from inside a directory barrel is not followed. None exist in 0.5.0 today — but Chat/index.d.ts is already a 47-line barrel of named re-exports, and a switch to barrel-of-barrels would silently drop ChatMessage and ChatMessageMetadata, the very names this PR added.

The only sanity check is set.size === 0. A parse that recovers 5 of 194 names passes and produces a wholly wrong artifact that CI then enforces.

A floor assertion tied to the resolved package version, or an else throw on an unresolvable star target, would make the invariant real. This is a ② path today — but it is the same class of silent drift the PR exists to remove, so it is worth closing here rather than later.

Astro-Han · 2026-08-26T18:13:19Z

@@ -431,6 +502,9 @@ export function renderAstryxSurfaceInventory(repoRoot = root) {
  lines.push(
    '- **blocker** — raw interactive control with an Astryx twin available (`button`/`input`/`select`).',
  );


P3 — the artifact is now a function of the installed dependency but records nothing about it.

@astryxdesign/core is exact-pinned (package.json:95, packages/ui/package.json:26, and the lockfile all at 0.5.0), so CI is reproducible — I checked, and the overrides entry only pins react-dom, it does not affect the version.

But the generated doc no longer says which version produced it, and regeneration silently depends on whatever is in the contributor's node_modules. Someone with a stale install regenerates a different artifact and has no way to see why CI disagrees. Emitting the resolved version in the header costs one line and makes the byte-diff self-explaining.

The file-level Astryx surface inventory could not see most of the design system. Its known-component set was a hand-maintained 47-name list while @astryxdesign/core ships ~190 components, so Timestamp, ChatMessage, Thumbnail and others were invisible — removing such a component from a file left its inventory row byte-identical and the coverage gate saw nothing. Severity also recognized only raw button/input/select, so a hand-rolled replacement for a shipped component scored aligned. Derive the component set by parsing the installed @astryxdesign/core declarations (hard error if the package is absent, so the artifact never silently regenerates with a partial set), add a reimplementation severity tier for a local component that shadows a shipped twin it does not import, and regenerate the committed inventory. Generated-by: Claude Opus 4.8 via Claude Code

liuxiaocs7 force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from 5439e35 to 329a92b Compare August 26, 2026 08:09

Astro-Han reviewed Aug 26, 2026

View reviewed changes

M4n5ter force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from 329a92b to 47a2dc2 Compare August 26, 2026 08:37

Astro-Han reviewed Aug 26, 2026

View reviewed changes

liuxiaocs7 force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from 47a2dc2 to 78a6335 Compare August 26, 2026 09:41

M4n5ter force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from 78a6335 to 1397483 Compare August 26, 2026 09:58

liuxiaocs7 force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from 1397483 to ba1e061 Compare August 26, 2026 18:10

Astro-Han reviewed Aug 26, 2026

View reviewed changes

github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026

liuxiaocs7 force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from ba1e061 to f362f07 Compare August 27, 2026 04:57

liuxiaocs7 force-pushed the liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo branch from f362f07 to 1182058 Compare August 27, 2026 05:50

github-actions Bot added effort/L Under 1000 readable lines and removed effort/M Under 500 readable lines labels Aug 27, 2026

Navigation Menu

Uh oh!

fix(build): derive Astryx surface inventory from @astryxdesign/core - #3883

fix(build): derive Astryx surface inventory from @astryxdesign/core#3883
liuxiaocs7 wants to merge 1 commit into
apache:mainfrom
liuxiaocs7:liuxiaocs7/fix-build-astryx-surface-inventory-cannot-see-mo

liuxiaocs7 commented Aug 26, 2026 •

edited

Loading

Uh oh!

Astro-Han left a comment

Uh oh!

Astro-Han left a comment

Uh oh!

Astro-Han left a comment

Uh oh!

Astro-Han Aug 26, 2026

Uh oh!

Astro-Han Aug 26, 2026

Uh oh!

Astro-Han Aug 26, 2026

Uh oh!

Astro-Han Aug 26, 2026

Uh oh!

Astro-Han Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

liuxiaocs7 commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Review response (thanks @codex)

AI use

Checklist

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

Uh oh!

Astro-Han left a comment

Choose a reason for hiding this comment

What this PR does

Verdict

The thread running through it

Mechanics

Note (ungraded)

Uh oh!

Astro-Han Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Astro-Han Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Astro-Han Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Astro-Han Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Astro-Han Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

liuxiaocs7 commented Aug 26, 2026 •

edited

Loading