feat(genui): add TypeDoc-driven A2UI catalog extractor#2521
Closed
PupilTong wants to merge 5 commits intolynx-family:mainfrom
Closed
feat(genui): add TypeDoc-driven A2UI catalog extractor#2521PupilTong wants to merge 5 commits intolynx-family:mainfrom
PupilTong wants to merge 5 commits intolynx-family:mainfrom
Conversation
PupilTong
requested review from
HuJean,
Sherry-hue,
fzx2666-fz and
gaoachao
as code owners
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
packages/genui/a2ui/package.json (1)
96-96: Prefer the extractor's workspacebinover invoking raw TS source.The build directly invokes
../a2ui-catalog-extractor/src/cli.tswithnode --experimental-strip-types, bypassing the package's declaredbinentry (a2ui-catalog-extractor→./dist/cli.js). While the current approach works—because the extractor source happens to conform to Node's type-stripping constraints and turbo's^buildordering ensures the extractor is compiled first—this pattern:
- Tightly couples the build to the sibling package's source-code layout, undermining the purpose of workspace dependencies and public entry points.
- Obscures the actual dependency chain: future refactors to the extractor's TypeScript syntax or import structure could silently break this build, even though they pass the extractor's own build.
Since
@lynx-js/a2ui-catalog-extractoris already declared as aworkspace:*devDep and itsbinentry is configured, use the built CLI:Suggested change
- "build": "node --experimental-strip-types ../a2ui-catalog-extractor/src/cli.ts generate --source ./src/catalog --out ./dist/catalog --tsconfig ./tsconfig.json --format legacy-shards && tsc -b" + "build": "a2ui-catalog-extractor generate --source ./src/catalog --out ./dist/catalog --tsconfig ./tsconfig.json --format legacy-shards && tsc -b"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/package.json` at line 96, The build script in the package.json currently calls the extractor's source via "node --experimental-strip-types ../a2ui-catalog-extractor/src/cli.ts", coupling to the sibling package layout; change the "build" npm script to invoke the extractor's declared bin (a2ui-catalog-extractor) instead so the workspace-provided CLI is used (e.g. replace the node invocation with "a2ui-catalog-extractor generate --source ./src/catalog --out ./dist/catalog --tsconfig ./tsconfig.json --format legacy-shards && tsc -b"), keeping the rest of the arguments and the trailing "tsc -b" intact.packages/genui/a2ui-catalog-extractor/test/cli.test.ts (1)
33-116: Temp directories are never removed.All three tests
fs.mkdtempunderos.tmpdir()and never unlink, so each CI run leaks directories (and, on the drift test, also a modifiedChip/catalog.json). Use atry { ... } finally { await fs.rm(outputDir, { recursive: true, force: true }); }around each test body, or extract a small helper.♻️ Proposed cleanup pattern
test('generate writes catalog shards to disk', async () => { const outputDir = await fs.mkdtemp( path.join(os.tmpdir(), 'a2ui-catalog-cli-generate-'), ); - - const { stdout } = await runCli([...]); - ... - expect(JSON.parse(generated)).toHaveProperty('Chip'); + try { + const { stdout } = await runCli([...]); + ... + expect(JSON.parse(generated)).toHaveProperty('Chip'); + } finally { + await fs.rm(outputDir, { recursive: true, force: true }); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/test/cli.test.ts` around lines 33 - 116, Each test creates a temporary outputDir via fs.mkdtemp and never removes it; wrap each test body that calls runCli (including the "generate", "check", and "drift" cases) in a try { ... } finally { await fs.rm(outputDir, { recursive: true, force: true }) } (or factor out a helper that creates the temp dir, yields it to the test, and always cleans up) so that outputDir (and any modified Chip/catalog.json) is removed after the test; refer to the existing fs.mkdtemp usage, the outputDir variable, and runCli invocation points to locate where to add the try/finally cleanup.packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts (2)
173-197: Temp directory is never cleaned up.
fs.mkdtempcreates a directory underos.tmpdir()that is never removed after the test. It's harmless but accumulates over repeated runs. Consider atry { ... } finally { await fs.rm(outputDir, { recursive: true, force: true }); }wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts` around lines 173 - 197, The test 'writeCatalogFiles and checkCatalogFiles round-trip generated shards' creates a temp dir with fs.mkdtemp (assigned to outputDir) but never removes it; wrap the test's runtime usage of outputDir in a try { ... } finally { await fs.rm(outputDir, { recursive: true, force: true }); } so the directory is always cleaned up after using extractCatalog, writeCatalogFiles and checkCatalogFiles; ensure the finally runs even on assertion failures.
106-131: Golden-baseline test couples to repo's realdist/path viaoutDir.
renderCatalogFilesonly usesoutDirto compute paths (no I/O), so this works today. Just be aware that theoutDirvalue here is effectively ignored by the assertion (you only read viapath.basename(path.dirname(...))); you could pass any placeholder outDir and get the same result, which would decouple the test frompackages/genui/a2ui/dist/catalog. Not a defect, just more robust.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts` around lines 106 - 131, The test for legacy A2UI catalog fixtures passes a real repository dist path into renderCatalogFiles via the outDir option, coupling the test to packages/genui/a2ui/dist/catalog even though renderCatalogFiles only uses outDir to compute file paths (no I/O); change the call that constructs renderedFiles to pass a neutral placeholder outDir (e.g., a tmp or dummy path) instead of path.join(workspaceRoot, 'packages/genui/a2ui/dist/catalog') so the assertions that use path.basename(path.dirname(renderedFile.path)) remain valid while decoupling the test from the repo-specific dist location; keep the rest of the test (extractCatalog, fixturePath lookup and content assertions) unchanged.packages/genui/a2ui/src/catalog/Button/index.tsx (1)
9-75: Override appears to duplicate what the extractor can infer from the TS type.The
actionfield's TS shape ({ event: { name: string; context?: Record<string, string | number | boolean | { path: string }> } }) already encodes everything the@a2uiSchemablock asserts — includingrequired: ['event'],required: ['name'], theoneOffor context values, and{ path: string }branches. Per the extractor instructions,@a2uiSchemashould be reserved for "node-local JSON Schema fragments that cannot be represented clearly with normal declarations." If the inferred schema already matches the fixture, dropping the override would keep this file leaner and reduce drift risk between the TS type and the hand-written JSON.Worth verifying by extracting without the override and diffing against the golden fixture; if they match, prefer deletion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Button/index.tsx` around lines 9 - 75, The `@a2uiSchema` JSON schema block on ButtonProps duplicates the TypeScript shape of the action property (action -> event -> name, context) and should be removed to avoid drift: delete the `@a2uiSchema` comment block that annotates ButtonProps.action (the JSON schema starting at the comment before action) so the extractor will infer the schema from the TypeScript declaration (ButtonProps.action / event.name / event.context); after removal, run the extractor and diff the generated schema against the golden fixture to verify they match, and only keep the override if there is a concrete mismatch.packages/genui/a2ui-catalog-extractor/src/cli.ts (1)
71-105: Consider catching top-level rejections for a friendlier CLI UX.
await main()at the top level will surface a raw unhandled-rejection stack (and still exit non-zero on Node 24) for expected failures like missing--source, invalid format, extractor errors, etc. Wrapping in a small handler gives users a single-line message and a clean exit code.🛠️ Suggested change
-await main(); +try { + await main(); +} catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/cli.ts` around lines 71 - 105, The top-level await on main() can surface raw unhandled-rejection stacks; replace the bare await main() with a top-level error handler that calls main() and catches any rejection, prints a single-line friendly error (e.g., use err instanceof Error ? err.message : String(err) with console.error), and sets process.exitCode = 1; locate the current top-level call to main() and change it to an invocation with .catch(...) (or a try/catch wrapper) so failures from parseCatalogOptions, extractCatalog, writeCatalogFiles, or checkCatalogFiles produce a clean, single-line CLI error and exit code.packages/genui/a2ui-catalog-extractor/src/extractor.ts (2)
383-401: Dead branch and duplicatedschema.requiredassignment.
requiredis populated only inside thefor (const property of properties)loop at lines 354-367, and that loop is the sole source of property-name collection. So whenpropertyNames.length === 0,requiredis guaranteed to be[], making theelse if (required.length > 0)branch unreachable. The two branches can collapse to a single check.♻️ Proposed simplification
- if (propertyNames.length > 0) { - schema.required = required; - } else if (required.length > 0) { - schema.required = required; - } - - if (additionalProperties === undefined) { - if (propertyNames.length > 0) { - schema.additionalProperties = false; - } - } else { + if (required.length > 0) { + schema.required = required; + } + + if (additionalProperties === undefined) { + if (propertyNames.length > 0) { + schema.additionalProperties = false; + } + } else { schema.additionalProperties = additionalProperties; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 383 - 401, The post-loop logic duplicates an impossible branch: since required is only filled inside the for (const property of properties) that also populates propertyNames, the else if (required.length > 0) branch can never run when propertyNames.length === 0; consolidate the required assignment by setting schema.required = required when propertyNames.length > 0 (remove the separate else-if) and simplify the additionalProperties handling to set schema.additionalProperties = false when additionalProperties === undefined && propertyNames.length > 0, otherwise assign the provided additionalProperties value; update the code paths referencing propertyNames, required, schema, and additionalProperties accordingly.
964-964: Use a fixed locale for deterministic output.
Array.prototype.sortwith defaultlocaleComparedepends on the runtime ICU locale, which can reorder components likezvs accented variants across machines/CI runners. Since the output is a golden-tested catalog, prefer a stable ordering.♻️ Suggested fix
- components.sort((left, right) => left.name.localeCompare(right.name)); + components.sort((left, right) => + left.name.localeCompare(right.name, 'en-US') + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` at line 964, The sort is nondeterministic across locales because it calls left.name.localeCompare(right.name) without a fixed locale; update the comparison used by components.sort to call localeCompare with an explicit, fixed locale (e.g., "en" or "en-US") and optional stable options (e.g., sensitivity) so component ordering is stable across environments; locate the components.sort(...) call and replace the comparator to use left.name.localeCompare(right.name, "en-US", {sensitivity: "base"}) (or similar fixed-locale/options) to ensure deterministic output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.md`:
- Around line 16-27: The markdown table row containing the literal text "A | B"
is malformed because the pipe is unescaped; update that table row so the left
cell reads the union text with the pipe escaped (e.g., "A \| B") and set the
right cell to indicate the generated schema mapping to oneOf (use the schema
token `oneOf`) so the row reads: left cell "A \| B" and right cell "`oneOf`".
In `@packages/genui/a2ui-catalog-extractor/SKILL.md`:
- Around line 30-32: The SKILL.md Implementation Note incorrectly claims
`@a2uiSchema` payloads are read from the TypeScript AST only; update the wording
to state the real dual-source behavior: TypeDoc is the primary source (see
src/docs.ts:buildPropertyDoc which uses
reflection.comment?.getTag('@a2uiSchema') and src/docs.ts:buildTypeDocIndex
which registers the blockTag), and src/extractor.ts:getAstPropertyDoc provides
an AST fallback for inline property declarations; rephrase to explicitly
instruct contributors to keep both paths (TypeDoc-first, AST-as-fallback) so
future edits won't remove the TypeDoc handling.
In `@packages/genui/a2ui-catalog-extractor/src/cli.ts`:
- Around line 54-57: Validate the format passed in values['format'] before
casting: ensure the value is one of the CatalogFormat string literals (e.g., the
allowed enum/union members) and if undefined default to 'legacy-shards'; if the
string is not a valid CatalogFormat, throw or exit with a clear error message.
Update the construction of extractOptions (and any code that reads
values['format']) to perform this check rather than an unchecked cast so
renderCatalogFiles won't silently treat unknown formats as legacy; reference the
ExtractCatalogOptions type and the CatalogFormat symbol when implementing the
guard and error path.
In `@packages/genui/a2ui-catalog-extractor/src/docs.ts`:
- Around line 69-95: parseScalarToken currently returns undefined for both empty
input and the literal token "undefined", causing buildPropertyDoc (which checks
defaultValue !== undefined) to silently drop `@default` undefined; also JSON.parse
errors are unwrapped. Fix by changing parseScalarToken to treat the literal
'undefined' as an explicit error (throw a clear, descriptive Error mentioning
the offending token and the caller parseScalarToken) instead of returning
undefined, and wrap the JSON.parse call (the branch that tests
startsWith('"')/startsWith('[')/startsWith('{')) in a try/catch that rethrows a
new Error including the original trimmed content and that it occurred while
parsing a default/@a2uiSchema token so callers can surface actionable messages;
update any unit tests to expect the new error behavior for the 'undefined' token
and for malformed JSON.
- Around line 187-192: The switch handling override keys (the block with cases
'const', 'default', 'deprecated', 'description', 'type' that currently just
continues) must validate the 'type' override against the allowed JsonSchema
types; instead of falling through for 'type', check that the provided value is
one of: 'array', 'boolean', 'number', 'object', 'string' and throw or report a
validation error if not. Update the branch in the same switch (the case 'type')
to perform this membership check on the override value and reject/raise an error
with a clear message referencing the invalid type value so typos like "String"
are caught before the catalog is emitted.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 744-747: parseTypeNode currently returns a silent permissive
fallback ({ type: 'string' }) for unknown/unhandled TypeScript nodes which can
hide schema errors; change it to surface diagnostics by throwing or recording a
warning that includes the source file path and the node text (use
typeNode.getText(sourceFile)) when no rule matches, and only allow the
permissive { type: 'string' } fallback when the source file's ScriptKind
indicates a permissive JSX/JS mode; update parseTypeNode (and any callers) to
either propagate the thrown diagnostic or consume/aggregate the warning into a
collected warnings list so unrecognized types are visible to the caller.
- Around line 1011-1046: The function checkCatalogFiles performs a direct string
comparison between file.content (from renderCatalogFiles) and the on-disk
content read with fs.readFile, which produces false mismatches on CRLF vs LF
platforms; normalize line endings on both sides before comparing (e.g., convert
both fileContent and file.content to use '\n' only) so mismatched, missing,
actual and expected are set from the normalized values; update the comparison in
checkCatalogFiles to compare normalizedFileContent !== normalizedExpectedContent
and assign actual/expected from the normalized strings.
- Around line 90-138: getComponentDeclarations currently only inspects
FunctionDeclaration and VariableStatement and will silently miss re-exports and
default export assignments; update getComponentDeclarations to detect
ts.isExportAssignment (export default Foo) and ts.isExportDeclaration with
namedBindings/specifiers (export { Foo }) within sourceFile.statements and, when
encountered, throw a clear, descriptive error (e.g. referencing the unsupported
export kind and the symbol name/text) so the extractor fails loudly; reference
the function name getComponentDeclarations and the AST kinds ts.ExportAssignment
and ts.ExportDeclaration in your change so the extractor explicitly handles or
rejects these patterns.
In `@packages/genui/a2ui-catalog-extractor/src/types.ts`:
- Line 39: The JsonSchema.enum field is typed too narrowly as string[] while
validateSchemaValue in docs.ts allows enum entries of null | boolean | number |
string; update the JsonSchema type to reflect the full set of allowed literal
types (e.g., enum?: Array<string | number | boolean | null>) so the TypeScript
type matches runtime validation and downstream uses like mergeSchema; locate the
JsonSchema definition in types.ts and adjust the enum member accordingly (or
alternatively, if you prefer stricter runtime behavior, change
validateSchemaValue in docs.ts to only accept strings and keep JsonSchema.enum
as string[] — pick one consistent approach and apply it across JsonSchema and
validateSchemaValue/mergeSchema).
In `@packages/genui/a2ui/src/catalog/Row/index.tsx`:
- Around line 15-23: The union type for justify includes 'stretch' but there's
no .distribution-stretch rule in Row/style.css and Column/style.css; either
remove 'stretch' from the justify union in Row/index.tsx (and any corresponding
Column type) to keep types in sync with available CSS classes, or add a
.distribution-stretch CSS rule to both Row/style.css and Column/style.css
implementing the same main-axis distribution behavior as the other
.distribution-* classes (mirror how .distribution-start/.distribution-center
etc. set justify-content) so the emitted class name distribution-stretch has a
backing style.
In `@packages/genui/a2ui/turbo.json`:
- Around line 11-12: Replace the relative cross-package inputs in
packages/genui/a2ui/turbo.json with the Turborepo root token: change the entries
referencing "../a2ui-catalog-extractor/src/**" and
"../a2ui-catalog-extractor/tsconfig*.json" to use "$TURBO_ROOT$" and the package
path (i.e., point to the packages/genui/a2ui-catalog-extractor/src/** and
packages/genui/a2ui-catalog-extractor/tsconfig*.json via $TURBO_ROOT$) so the
Turbo cache and artifact resolution follow the repository convention used
elsewhere.
---
Nitpick comments:
In `@packages/genui/a2ui-catalog-extractor/src/cli.ts`:
- Around line 71-105: The top-level await on main() can surface raw
unhandled-rejection stacks; replace the bare await main() with a top-level error
handler that calls main() and catches any rejection, prints a single-line
friendly error (e.g., use err instanceof Error ? err.message : String(err) with
console.error), and sets process.exitCode = 1; locate the current top-level call
to main() and change it to an invocation with .catch(...) (or a try/catch
wrapper) so failures from parseCatalogOptions, extractCatalog,
writeCatalogFiles, or checkCatalogFiles produce a clean, single-line CLI error
and exit code.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 383-401: The post-loop logic duplicates an impossible branch:
since required is only filled inside the for (const property of properties) that
also populates propertyNames, the else if (required.length > 0) branch can never
run when propertyNames.length === 0; consolidate the required assignment by
setting schema.required = required when propertyNames.length > 0 (remove the
separate else-if) and simplify the additionalProperties handling to set
schema.additionalProperties = false when additionalProperties === undefined &&
propertyNames.length > 0, otherwise assign the provided additionalProperties
value; update the code paths referencing propertyNames, required, schema, and
additionalProperties accordingly.
- Line 964: The sort is nondeterministic across locales because it calls
left.name.localeCompare(right.name) without a fixed locale; update the
comparison used by components.sort to call localeCompare with an explicit, fixed
locale (e.g., "en" or "en-US") and optional stable options (e.g., sensitivity)
so component ordering is stable across environments; locate the
components.sort(...) call and replace the comparator to use
left.name.localeCompare(right.name, "en-US", {sensitivity: "base"}) (or similar
fixed-locale/options) to ensure deterministic output.
In `@packages/genui/a2ui-catalog-extractor/test/cli.test.ts`:
- Around line 33-116: Each test creates a temporary outputDir via fs.mkdtemp and
never removes it; wrap each test body that calls runCli (including the
"generate", "check", and "drift" cases) in a try { ... } finally { await
fs.rm(outputDir, { recursive: true, force: true }) } (or factor out a helper
that creates the temp dir, yields it to the test, and always cleans up) so that
outputDir (and any modified Chip/catalog.json) is removed after the test; refer
to the existing fs.mkdtemp usage, the outputDir variable, and runCli invocation
points to locate where to add the try/finally cleanup.
In `@packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts`:
- Around line 173-197: The test 'writeCatalogFiles and checkCatalogFiles
round-trip generated shards' creates a temp dir with fs.mkdtemp (assigned to
outputDir) but never removes it; wrap the test's runtime usage of outputDir in a
try { ... } finally { await fs.rm(outputDir, { recursive: true, force: true });
} so the directory is always cleaned up after using extractCatalog,
writeCatalogFiles and checkCatalogFiles; ensure the finally runs even on
assertion failures.
- Around line 106-131: The test for legacy A2UI catalog fixtures passes a real
repository dist path into renderCatalogFiles via the outDir option, coupling the
test to packages/genui/a2ui/dist/catalog even though renderCatalogFiles only
uses outDir to compute file paths (no I/O); change the call that constructs
renderedFiles to pass a neutral placeholder outDir (e.g., a tmp or dummy path)
instead of path.join(workspaceRoot, 'packages/genui/a2ui/dist/catalog') so the
assertions that use path.basename(path.dirname(renderedFile.path)) remain valid
while decoupling the test from the repo-specific dist location; keep the rest of
the test (extractCatalog, fixturePath lookup and content assertions) unchanged.
In `@packages/genui/a2ui/package.json`:
- Line 96: The build script in the package.json currently calls the extractor's
source via "node --experimental-strip-types
../a2ui-catalog-extractor/src/cli.ts", coupling to the sibling package layout;
change the "build" npm script to invoke the extractor's declared bin
(a2ui-catalog-extractor) instead so the workspace-provided CLI is used (e.g.
replace the node invocation with "a2ui-catalog-extractor generate --source
./src/catalog --out ./dist/catalog --tsconfig ./tsconfig.json --format
legacy-shards && tsc -b"), keeping the rest of the arguments and the trailing
"tsc -b" intact.
In `@packages/genui/a2ui/src/catalog/Button/index.tsx`:
- Around line 9-75: The `@a2uiSchema` JSON schema block on ButtonProps duplicates
the TypeScript shape of the action property (action -> event -> name, context)
and should be removed to avoid drift: delete the `@a2uiSchema` comment block that
annotates ButtonProps.action (the JSON schema starting at the comment before
action) so the extractor will infer the schema from the TypeScript declaration
(ButtonProps.action / event.name / event.context); after removal, run the
extractor and diff the generated schema against the golden fixture to verify
they match, and only keep the override if there is a concrete mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42b5ecc4-6e73-4ccb-8277-ca098b42e897
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.github/a2ui-catalog-extractor.instructions.mdpackages/genui/a2ui-catalog-extractor/README.mdpackages/genui/a2ui-catalog-extractor/SKILL.mdpackages/genui/a2ui-catalog-extractor/agents/openai.yamlpackages/genui/a2ui-catalog-extractor/eslint.config.jspackages/genui/a2ui-catalog-extractor/package.jsonpackages/genui/a2ui-catalog-extractor/references/a2ui-v0.9-schema.mdpackages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.mdpackages/genui/a2ui-catalog-extractor/rslib.config.tspackages/genui/a2ui-catalog-extractor/rstest.config.tspackages/genui/a2ui-catalog-extractor/src/cli.tspackages/genui/a2ui-catalog-extractor/src/docs.tspackages/genui/a2ui-catalog-extractor/src/extractor.tspackages/genui/a2ui-catalog-extractor/src/index.tspackages/genui/a2ui-catalog-extractor/src/types.tspackages/genui/a2ui-catalog-extractor/test/cli.test.tspackages/genui/a2ui-catalog-extractor/test/extract-catalog.test.tspackages/genui/a2ui-catalog-extractor/test/fixtures/jsx/catalog/Badge/index.jsxpackages/genui/a2ui-catalog-extractor/test/fixtures/jsx/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Button/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Card/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/CheckBox/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Column/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Divider/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Image/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/List/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/RadioGroup/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Row/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Text/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid/catalog/Broken/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx/catalog/Chip/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/tsconfig.build.jsonpackages/genui/a2ui-catalog-extractor/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/tsdoc.jsonpackages/genui/a2ui-catalog-extractor/turbo.jsonpackages/genui/a2ui/README.mdpackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Card/index.tsxpackages/genui/a2ui/src/catalog/CheckBox/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/catalog/Divider/index.tsxpackages/genui/a2ui/src/catalog/Image/index.tsxpackages/genui/a2ui/src/catalog/List/index.tsxpackages/genui/a2ui/src/catalog/RadioGroup/index.tsxpackages/genui/a2ui/src/catalog/Row/index.tsxpackages/genui/a2ui/src/catalog/Text/index.tsxpackages/genui/a2ui/tools/catalog_generator.tspackages/genui/a2ui/turbo.jsonpackages/genui/tsconfig.json
💤 Files with no reviewable changes (1)
- packages/genui/a2ui/tools/catalog_generator.ts
Comment on lines
15
to
23
| justify?: | ||
| | 'start' | ||
| | 'center' | ||
| | 'end' | ||
| | 'stretch' | ||
| | 'spaceBetween' | ||
| | 'spaceAround' | ||
| | 'spaceEvenly' | ||
| | 'stretch'; | ||
| | 'spaceEvenly'; | ||
| align?: 'start' | 'center' | 'end' | 'stretch'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify both Row and Column styles define distribution-stretch (and related) classes.
fd -a 'style.css' packages/genui/a2ui/src/catalog | xargs -I {} sh -c 'echo "=== {} ==="; cat "{}"'
rg -nP '\bdistribution-(stretch|spaceEvenly|spaceBetween|spaceAround|start|center|end)\b' packages/genui/a2uiRepository: lynx-family/lynx-stack
Length of output: 9863
Add distribution-stretch CSS rule to Row and Column styles, or remove 'stretch' from the justify union.
The code adds 'stretch' to the justify union type, but packages/genui/a2ui/src/catalog/Row/style.css and packages/genui/a2ui/src/catalog/Column/style.css do not define a corresponding .distribution-stretch CSS class. When justify: 'stretch' is used, the renderer will emit the class name distribution-stretch with no CSS rule backing it, causing the element to render without any main-axis distribution styling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui/src/catalog/Row/index.tsx` around lines 15 - 23, The
union type for justify includes 'stretch' but there's no .distribution-stretch
rule in Row/style.css and Column/style.css; either remove 'stretch' from the
justify union in Row/index.tsx (and any corresponding Column type) to keep types
in sync with available CSS classes, or add a .distribution-stretch CSS rule to
both Row/style.css and Column/style.css implementing the same main-axis
distribution behavior as the other .distribution-* classes (mirror how
.distribution-start/.distribution-center etc. set justify-content) so the
emitted class name distribution-stretch has a backing style.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/genui/a2ui-catalog-extractor/src/extractor.ts (2)
1041-1079:checkCatalogFilesconflates all read errors with "missing".The bare
catch { missing.push(file.path) }lumps everyfs.readFilefailure — including permission errors, EISDIR, EIO, etc. — into themissingbucket. Debugging a CIcatalog:checkfailure on a wonky filesystem/permissions setup will be confusing because the tool will say the file is missing when it isn't. Consider narrowing the catch toENOENTand rethrowing other errors.🔧 Narrow to ENOENT
- } catch { - missing.push(file.path); - } + } catch (error) { + if ((error as NodeJS.ErrnoException)?.code === 'ENOENT') { + missing.push(file.path); + continue; + } + throw error; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 1041 - 1079, The catch in checkCatalogFiles currently treats any fs.readFile failure as "missing"; change the error handling inside the for loop around await fs.readFile(file.path, ...) so that you only push file.path into the missing array when the error code is ENOENT, and rethrow (or propagate) any other errors (e.g., permission/EISDIR/EIO) so they surface instead of being misclassified; keep all other logic (mismatched, actual/expected assignment, and returned CheckCatalogFilesResult) unchanged.
1008-1025:renderCatalogFilestrustsresult.catalogto be populated.
result.catalogis typed as optional but is dereferenced unconditionally for thea2ui-catalogformat. If a caller (e.g., a library consumer) constructs anExtractCatalogResultwithformat: 'a2ui-catalog'but omitscatalog,JSON.stringify(undefined, null, 2)returns the primitive valueundefined, and the template literal writes a literal"undefined\n"file with no error. A narrow guard would fail fast with a useful message instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 1008 - 1025, The function renderCatalogFiles currently assumes result.catalog exists when result.format === 'a2ui-catalog' and will emit the literal "undefined" if it's missing; update renderCatalogFiles to validate that result.catalog is defined when format is 'a2ui-catalog' and throw a clear, fast-failing Error (or return a rejected Promise if async) otherwise; reference the symbols result, renderCatalogFiles, ExtractCatalogResult and catalog in the error message so callers know they passed an invalid ExtractCatalogResult; keep behavior for non-'a2ui-catalog' paths unchanged.packages/genui/a2ui-catalog-extractor/src/docs.ts (1)
316-336: Multiple@a2uiSchematags silently take the last one.
getAstPropertyDociterates all@a2uiSchematags and lets the last one overwriteschemaOverridewithout warning. If a developer accidentally adds two of these tags, the earlier override is dropped silently. Consider either breaking after the first tag or throwing on duplicates for better debuggability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/docs.ts` around lines 316 - 336, In getAstPropertyDoc, the loop over comment tags for `@a2uiSchema` currently lets later tags overwrite schemaOverride silently; update getAstPropertyDoc so it detects multiple `@a2uiSchema` tags and either (a) stops after the first tag by breaking once schemaOverride is set, or (b) throws a clear error when a second `@a2uiSchema` is encountered; locate the tag iteration in getAstPropertyDoc (where schemaOverride is assigned) and add the duplicate-detection logic and an early break or throw to prevent silent overwrites.packages/genui/a2ui-catalog-extractor/src/cli.ts (1)
115-120: Consider printing usage on parse errors.When
parseCatalogOptionsthrows (missing--source/--out, unknown option understrict: true, unsupported--format), onlyerror.messageis printed and the CLI exits with 1. InvokingprintUsage()in the catch for argument-shaped errors would make the CLI more discoverable, but this is non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/cli.ts` around lines 115 - 120, Catch block should call printUsage() for argument/parse errors so users see help; modify the catch around await main() to detect errors coming from CLI parsing (thrown by parseCatalogOptions) — e.g., check error originates from parseCatalogOptions (by error.name, a custom property you add when throwing, or by matching known parse error messages like missing --source/--out/unsupported --format/unknown option) and invoke printUsage() before logging the error and setting process.exitCode = 1; ensure functions parseCatalogOptions and printUsage are referenced so you locate where to add the detection and call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 114-120: The current blanket rejection of ts.isExportDeclaration
blocks type-only and star re-exports; change the condition so you only throw for
export declarations that introduce value bindings: check
ts.isExportDeclaration(statement) AND that statement.isTypeOnly is false AND
that statement.exportClause is present (meaning it's a named re-export of
values), then throw the Error using the existing message (reference symbols:
ts.isExportDeclaration, ExportDeclaration, statement.isTypeOnly,
statement.exportClause, statement.getText(sourceFile), sourceFile.fileName).
- Around line 515-532: collectInterfaceProperties currently only uses
declaration.members and thus omits inherited properties from heritage clauses;
update it to resolve each heritage clause type via context.localDeclarations and
for each resolved base declaration call parseNamedDeclaration (or recursively
call collectInterfaceProperties) to collect and merge inherited members into the
CollectedTypeElements before calling collectTypeElementProperties, honoring the
parseState.seen set to avoid cycles; if a heritage clause cannot be resolved
through context.localDeclarations, throw a clear error naming the unresolved
base type to prompt an `@a2uiSchema` override or simplification.
---
Nitpick comments:
In `@packages/genui/a2ui-catalog-extractor/src/cli.ts`:
- Around line 115-120: Catch block should call printUsage() for argument/parse
errors so users see help; modify the catch around await main() to detect errors
coming from CLI parsing (thrown by parseCatalogOptions) — e.g., check error
originates from parseCatalogOptions (by error.name, a custom property you add
when throwing, or by matching known parse error messages like missing
--source/--out/unsupported --format/unknown option) and invoke printUsage()
before logging the error and setting process.exitCode = 1; ensure functions
parseCatalogOptions and printUsage are referenced so you locate where to add the
detection and call.
In `@packages/genui/a2ui-catalog-extractor/src/docs.ts`:
- Around line 316-336: In getAstPropertyDoc, the loop over comment tags for
`@a2uiSchema` currently lets later tags overwrite schemaOverride silently; update
getAstPropertyDoc so it detects multiple `@a2uiSchema` tags and either (a) stops
after the first tag by breaking once schemaOverride is set, or (b) throws a
clear error when a second `@a2uiSchema` is encountered; locate the tag iteration
in getAstPropertyDoc (where schemaOverride is assigned) and add the
duplicate-detection logic and an early break or throw to prevent silent
overwrites.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 1041-1079: The catch in checkCatalogFiles currently treats any
fs.readFile failure as "missing"; change the error handling inside the for loop
around await fs.readFile(file.path, ...) so that you only push file.path into
the missing array when the error code is ENOENT, and rethrow (or propagate) any
other errors (e.g., permission/EISDIR/EIO) so they surface instead of being
misclassified; keep all other logic (mismatched, actual/expected assignment, and
returned CheckCatalogFilesResult) unchanged.
- Around line 1008-1025: The function renderCatalogFiles currently assumes
result.catalog exists when result.format === 'a2ui-catalog' and will emit the
literal "undefined" if it's missing; update renderCatalogFiles to validate that
result.catalog is defined when format is 'a2ui-catalog' and throw a clear,
fast-failing Error (or return a rejected Promise if async) otherwise; reference
the symbols result, renderCatalogFiles, ExtractCatalogResult and catalog in the
error message so callers know they passed an invalid ExtractCatalogResult; keep
behavior for non-'a2ui-catalog' paths unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5983757f-cda3-4a44-bad8-95c2a3d1f268
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.github/a2ui-catalog-extractor.instructions.mdpackages/genui/a2ui-catalog-extractor/SKILL.mdpackages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.mdpackages/genui/a2ui-catalog-extractor/src/cli.tspackages/genui/a2ui-catalog-extractor/src/docs.tspackages/genui/a2ui-catalog-extractor/src/extractor.tspackages/genui/a2ui-catalog-extractor/src/types.tspackages/genui/a2ui-catalog-extractor/test/cli.test.tspackages/genui/a2ui-catalog-extractor/test/extract-catalog.test.tspackages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/catalog/Loose/index.jsxpackages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/catalog/UndefinedDefault/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/catalog/DefaultExport/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/catalog/NamedExport/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/catalog/BrokenJson/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-schema-type/catalog/BrokenSchemaType/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-schema-type/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/catalog/Fancy/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/tsconfig.jsonpackages/genui/a2ui/turbo.json
✅ Files skipped from review due to trivial changes (17)
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/catalog/Fancy/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/catalog/UndefinedDefault/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/catalog/Loose/index.jsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/catalog/NamedExport/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-schema-type/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-schema-type/catalog/BrokenSchemaType/index.tsx
- packages/genui/a2ui/turbo.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/catalog/DefaultExport/index.tsx
- packages/genui/a2ui-catalog-extractor/SKILL.md
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/catalog/BrokenJson/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/tsconfig.json
- .github/a2ui-catalog-extractor.instructions.md
- packages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.md
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/genui/a2ui-catalog-extractor/test/cli.test.ts
- packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts
Comment on lines
+114
to
+120
| if (ts.isExportDeclaration(statement)) { | ||
| throw new Error( | ||
| `Unsupported component export in ${sourceFile.fileName}: re-exports like "${ | ||
| statement.getText(sourceFile) | ||
| }" are not supported. Use a direct exported function or const declaration instead.`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Blanket ExportDeclaration rejection also blocks type-only re-exports.
ts.isExportDeclaration matches every export { ... } [from '...'] and export * from '...' — including type-only re-exports such as export type { ButtonProps } from './Button.tsx';. If an index.tsx just happens to re-export a type alongside the component definition, extraction will throw before the component is even considered. Consider narrowing the rejection to export declarations that contribute value bindings (skip isTypeOnly exports and pure export * side-effect statements that don't introduce component names).
🔧 Suggested narrowing
- if (ts.isExportDeclaration(statement)) {
+ if (ts.isExportDeclaration(statement) && !statement.isTypeOnly) {
throw new Error(
`Unsupported component export in ${sourceFile.fileName}: re-exports like "${
statement.getText(sourceFile)
}" are not supported. Use a direct exported function or const declaration instead.`,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 114 -
120, The current blanket rejection of ts.isExportDeclaration blocks type-only
and star re-exports; change the condition so you only throw for export
declarations that introduce value bindings: check
ts.isExportDeclaration(statement) AND that statement.isTypeOnly is false AND
that statement.exportClause is present (meaning it's a named re-export of
values), then throw the Error using the existing message (reference symbols:
ts.isExportDeclaration, ExportDeclaration, statement.isTypeOnly,
statement.exportClause, statement.getText(sourceFile), sourceFile.fileName).
Comment on lines
+515
to
+532
| function collectInterfaceProperties( | ||
| declaration: ts.InterfaceDeclaration, | ||
| context: SourceContext, | ||
| parseState: ParseState, | ||
| ): CollectedTypeElements { | ||
| const typeDocRecord = getTypeDocRecord( | ||
| context.typeDocIndex, | ||
| context.filePath, | ||
| declaration.name.text, | ||
| ); | ||
|
|
||
| return collectTypeElementProperties( | ||
| declaration.members, | ||
| context, | ||
| parseState, | ||
| typeDocRecord, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether any catalog component props type uses `extends` so the impact can be assessed.
rg -nP --type=ts -C2 '\binterface\s+\w+Props\b[^{]*\bextends\b' packages/genui/a2ui/srcRepository: lynx-family/lynx-stack
Length of output: 4514
🏁 Script executed:
#!/bin/bash
# First, let's look at the function in question and surrounding context
head -n 600 packages/genui/a2ui-catalog-extractor/src/extractor.ts | tail -n 150Repository: lynx-family/lynx-stack
Length of output: 4005
🏁 Script executed:
#!/bin/bash
# Search for how heritage clauses might be handled
rg -n "heritage|extends|baseTypes" packages/genui/a2ui-catalog-extractor/src/extractor.tsRepository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Find GenericComponentProps definition
rg -n "GenericComponentProps" packages/genui/a2ui/src --type=tsRepository: lynx-family/lynx-stack
Length of output: 2448
🏁 Script executed:
#!/bin/bash
# Check if there's any special handling for interface inheritance
rg -n "heritageClause|getBaseTypes" packages/genui/a2ui-catalog-extractor/src/Repository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Read GenericComponentProps definition to see what's being missed
cat -n packages/genui/a2ui/src/core/types.ts | sed -n '77,120p'Repository: lynx-family/lynx-stack
Length of output: 364
🏁 Script executed:
#!/bin/bash
# Verify that declaration.members is the only source being used - check collectTypeElementProperties signature
rg -n "function collectTypeElementProperties|const collectTypeElementProperties" packages/genui/a2ui-catalog-extractor/src/extractor.ts -A 5Repository: lynx-family/lynx-stack
Length of output: 279
🏁 Script executed:
#!/bin/bash
# Check the full collectTypeElementProperties function to see what it does
rg -n "function collectTypeElementProperties" packages/genui/a2ui-catalog-extractor/src/extractor.ts -A 40Repository: lynx-family/lynx-stack
Length of output: 1349
🏁 Script executed:
#!/bin/bash
# Check if there's any handling of heritageClause in collectInterfaceProperties or nearby
rg -n "heritageClause|heritage" packages/genui/a2ui-catalog-extractor/src/extractor.tsRepository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if InterfaceDeclaration.members includes inherited members or if heritageClause is separate
rg -n "declaration.members|heritageClause" packages/genui/a2ui-catalog-extractor/src/extractor.tsRepository: lynx-family/lynx-stack
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Confirm TypeScript InterfaceDeclaration structure - check if members includes heritage
# Let's look for documentation or understanding of how heritageClauses work
rg -n "InterfaceDeclaration" packages/genui/a2ui-catalog-extractor/src/extractor.ts -B 2 -A 2Repository: lynx-family/lynx-stack
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Check if any of the 10 affected components have their own properties defined
# to understand what's being captured vs what's missing
cat -n packages/genui/a2ui/src/catalog/Text/index.tsx | sed -n '8,20p'Repository: lynx-family/lynx-stack
Length of output: 479
🏁 Script executed:
#!/bin/bash
# Check TypeScript documentation behavior - verify declaration.members doesn't include heritage
# Let's see if there are any test files or examples showing how heritage is handled
find packages/genui/a2ui-catalog-extractor -name "*.test.ts" -o -name "*.spec.ts" | head -5Repository: lynx-family/lynx-stack
Length of output: 188
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for collectInterfaceProperties that show expected behavior
rg -n "collectInterfaceProperties" packages/genui/a2ui-catalog-extractor --type=ts -B 2 -A 2Repository: lynx-family/lynx-stack
Length of output: 915
🏁 Script executed:
#!/bin/bash
# Let's verify: does the generated schema for TextProps include 'surface' from GenericComponentProps?
# Find any generated schemas or test outputs
find packages/genui/a2ui-catalog-extractor -name "*.json" -o -name "*schema*" | head -10Repository: lynx-family/lynx-stack
Length of output: 860
🏁 Script executed:
#!/bin/bash
# Check one of the legacy baseline catalogs to see if base properties are captured
cat packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Text/catalog.json 2>/dev/null || cat packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Button/catalog.jsonRepository: lynx-family/lynx-stack
Length of output: 787
🏁 Script executed:
#!/bin/bash
# Check the test file to see if it validates that base properties are present
rg -n "surface|id|setValue|GenericComponentProps" packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts -B 2 -A 2Repository: lynx-family/lynx-stack
Length of output: 2482
🏁 Script executed:
#!/bin/bash
# Verify the issue by checking another baseline catalog
cat packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Button/catalog.jsonRepository: lynx-family/lynx-stack
Length of output: 1811
🏁 Script executed:
#!/bin/bash
# Search for any heritage clause processing in the codebase
rg -n "heritageClause" packages/genui/a2ui-catalog-extractor/src/ -A 3Repository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if there's any handling in parseNamedDeclaration for resolving base interfaces
rg -n "getBaseTypeOfLiteralType|getResolvedType|resolveTypeReference" packages/genui/a2ui-catalog-extractor/src/extractor.tsRepository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Let's check the test file to see if there's any test specifically for inherited interfaces
rg -n "extends|inherit" packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts -B 2 -A 2Repository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if GenericComponentProps is defined as an interface and is it in localDeclarations
rg -n "GenericComponentProps" packages/genui/a2ui-catalog-extractor/test/ -lRepository: lynx-family/lynx-stack
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Check the full TextProps baseline to confirm what's included
cat packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Text/catalog.jsonRepository: lynx-family/lynx-stack
Length of output: 787
🏁 Script executed:
#!/bin/bash
# Verify that GenericComponentProps is NOT being resolved - check if there's a test fixture with it
find packages/genui/a2ui-catalog-extractor/test/fixtures -name "*.ts" -o -name "*.tsx" | xargs grep -l "extends.*Props" | head -3Repository: lynx-family/lynx-stack
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Check the Chip component in test fixtures to see if it extends something
cat packages/genui/a2ui-catalog-extractor/test/fixtures/tsx/catalog/Chip/index.tsxRepository: lynx-family/lynx-stack
Length of output: 773
🏁 Script executed:
#!/antml:
# Check if there's a corresponding baseline for Chip
cat packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Chip/catalog.json 2>/dev/null || echo "No Chip baseline found"Repository: lynx-family/lynx-stack
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Search for any test case that validates inheritance or base types
rg -n "extends|base|inherit" packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts -C 3Repository: lynx-family/lynx-stack
Length of output: 393
🏁 Script executed:
#!/bin/bash
# Check the test to see if it validates schemas against the baselines
cat packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts | sed -n '190,220p'Repository: lynx-family/lynx-stack
Length of output: 1016
🏁 Script executed:
#!/bin/bash
# Verify that GenericComponentProps properties (id, surface, sendAction) are NOT in any baseline
rg -n '"id"|"surface"|"sendAction"' packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/*/catalog.jsonRepository: lynx-family/lynx-stack
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check what files are in the test fixtures that are being extracted
ls -la packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Repository: lynx-family/lynx-stack
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Final confirmation: Check TypeScript's InterfaceDeclaration structure
# to confirm declaration.members does NOT include inherited properties
python3 << 'EOF'
# TypeScript fact: InterfaceDeclaration.members contains only direct members,
# not inherited members. Heritage clauses are in declaration.heritageClauses
# Let's verify by examining the code flow
import re
# Read extractor.ts to trace the flow
with open('packages/genui/a2ui-catalog-extractor/src/extractor.ts', 'r') as f:
content = f.read()
# Find collectInterfaceProperties function
match = re.search(r'function collectInterfaceProperties\([^)]*\)[^{]*\{[^}]*\}', content, re.DOTALL)
if match:
func = match.group(0)
print("collectInterfaceProperties implementation:")
print(func[:500])
print("\n--- KEY OBSERVATION ---")
if 'declaration.members' in func and 'heritageClause' not in func:
print("✓ CONFIRMED: Uses declaration.members WITHOUT heritageClause handling")
print("✓ This means inherited properties from 'extends' clauses are NOT processed")
if 'declaration.heritageClauses' not in func:
print("✓ CONFIRMED: No access to heritageClauses at all")
EOFRepository: lynx-family/lynx-stack
Length of output: 726
🏁 Script executed:
#!/bin/bash
# One final check: does the test fixture have GenericComponentProps defined in the same file?
# If so, the inheritance could be resolved locally
grep -n "interface GenericComponentProps" packages/genui/a2ui-catalog-extractor/test/fixtures/tsx/catalog/Chip/index.tsxRepository: lynx-family/lynx-stack
Length of output: 102
Heritage clause properties from extended interfaces are silently omitted from extracted schemas.
collectInterfaceProperties uses only declaration.members, which excludes inherited properties from interface heritage clauses. All 10 catalog components extend GenericComponentProps (which defines id, surface, setValue, sendAction, dataContextPath), yet these base properties never appear in the generated catalog schemas. The extractor produces incomplete, silent failures with no diagnostic.
Options:
- Resolve each heritage clause via
context.localDeclarations, recursively collect inherited members throughparseNamedDeclaration, and merge with direct members (respectingseento prevent cycles). - If unresolved, throw a clear error naming the base type so authors either simplify or add an
@a2uiSchemaoverride.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 515 -
532, collectInterfaceProperties currently only uses declaration.members and thus
omits inherited properties from heritage clauses; update it to resolve each
heritage clause type via context.localDeclarations and for each resolved base
declaration call parseNamedDeclaration (or recursively call
collectInterfaceProperties) to collect and merge inherited members into the
CollectedTypeElements before calling collectTypeElementProperties, honoring the
parseState.seen set to avoid cycles; if a heritage clause cannot be resolved
through context.localDeclarations, throw a clear error naming the unresolved
base type to prompt an `@a2uiSchema` override or simplification.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/genui/a2ui-catalog-extractor/src/extractor.ts (2)
516-533:⚠️ Potential issue | 🟠 MajorHeritage clause properties from extended interfaces are still silently omitted.
collectInterfacePropertiesonly readsdeclaration.members, so inherited members fromextendsclauses are dropped. All 10 A2UI catalog components extendGenericComponentProps; the extractor silently produces schemas missing any props declared on the base interface (e.g.id,surface,setValue,sendAction,dataContextPath). FilteringGENERIC_PROPSinbuildObjectSchemaat Line 378 confirms inheritance was intended to be processed — but the walk never actually pulls in the base-type members. The golden-parity tests only cover what the baseline already contains, so this regresses quietly.Resolve each
declaration.heritageClauses[*].typesthroughcontext.localDeclarations, recursively parse viaparseNamedDeclaration(honoringparseState.seento prevent cycles), and merge members with direct ones beforecollectTypeElementProperties. If a base type cannot be resolved locally, throw a clear error naming the unresolved base so authors can add an@a2uiSchemaoverride or flatten the declaration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 516 - 533, collectInterfaceProperties currently only uses declaration.members so inherited props from declaration.heritageClauses are dropped; update collectInterfaceProperties to iterate declaration.heritageClauses[*].types, resolve each type via context.localDeclarations, and for each resolved declaration call parseNamedDeclaration (respecting parseState.seen to avoid cycles) to obtain its members, then merge those inherited members with the direct declaration.members before calling collectTypeElementProperties; if a heritage type cannot be resolved locally, throw a clear error naming the unresolved base type so callers can add an `@a2uiSchema` override or flatten the declaration (this aligns with buildObjectSchema's GENERIC_PROPS filtering expectations).
115-121:⚠️ Potential issue | 🟡 MinorBlanket
ExportDeclarationrejection still blocks type-only and side-effect re-exports.
ts.isExportDeclarationmatches everyexport { ... } [from '...']andexport * from '...', including type-only re-exports (e.g.export type { ButtonProps } from './Button.tsx';). Anindex.tsxthat re-exports a prop type alongside its component definition will throw before the component is ever considered. Narrow the rejection to value re-exports only.🔧 Suggested narrowing
- if (ts.isExportDeclaration(statement)) { + if (ts.isExportDeclaration(statement) && !statement.isTypeOnly && statement.exportClause) { throw new Error( `Unsupported component export in ${sourceFile.fileName}: re-exports like "${ statement.getText(sourceFile) }" are not supported. Use a direct exported function or const declaration instead.`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 115 - 121, The current blanket ts.isExportDeclaration check rejects all export declarations including type-only re-exports; change it to only throw for value re-exports by checking statement.moduleSpecifier and that the export is not type-only. Concretely, update the condition to something like: if (ts.isExportDeclaration(statement) && statement.moduleSpecifier && !statement.isTypeOnly) { throw new Error(...) } so that type-only exports (statement.isTypeOnly) and local export clauses are allowed, while cross-module value re-exports still produce the error referencing sourceFile.fileName and statement.getText(sourceFile).
🧹 Nitpick comments (2)
packages/genui/a2ui-catalog-extractor/src/extractor.ts (2)
399-414: Minor code-smell: unreachable branch and redundant allocations inbuildObjectSchema.A couple of small cleanups in this block:
- The
else if (required.length > 0)branch at Line 412 is unreachable.requiredis only appended inside the loop overproperties, which also populatespropertyMap; sopropertyNames.length === 0impliesrequired.length === 0.- The
schemaobject initialized at Line 372 is never used by the top-level return path (Line 399-404) other than as a staging area forschema.properties. FortopLevel, you can return{ properties: propertyMap, required }directly.additionalPropertiesat Line 375 is just an alias forexplicitAdditionalProperties; using the parameter directly removes the indirection.Not behavior-affecting; purely readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 399 - 414, In buildObjectSchema: remove the unreachable else-if and redundant allocations by returning directly for the topLevel case using the constructed propertyMap and required array (i.e., return { properties: propertyMap, required }) instead of relying on the pre-initialized schema object; eliminate the useless alias additionalProperties and use explicitAdditionalProperties directly; also drop the unreachable branch that checks required.length > 0 after propertyNames is empty and simplify the logic that assigns schema.required when propertyNames.length > 0 to assign required onto schema (or the returned object) only when there are properties.
805-832:getComponentEntryFilesdoes serialfs.statper candidate; consider usingreaddiron each subdirectory instead.For each subdirectory you probe up to 4 filenames via
fs.statinside a loop. A singlefs.readdir(path.join(sourceDir, entry.name))returns the directory listing in one syscall and avoids the try/catch-per-candidate dance. Given catalog sizes are small this isn't a hot path, but the current shape couples correctness to EXT permissions (silent catches swallow non-ENOENT errors like EACCES, which would then look like "component has no entry file" rather than a permission problem).🔧 Sketch
for (const entry of directoryEntries.sort((l, r) => l.name.localeCompare(r.name))) { if (!entry.isDirectory()) continue; - for (const candidate of indexCandidates) { - const candidatePath = path.join(sourceDir, entry.name, candidate); - try { - const stats = await fs.stat(candidatePath); - if (stats.isFile()) { - entryFiles.push(candidatePath); - break; - } - } catch { - // Ignore missing entry candidates. - } - } + const dirPath = path.join(sourceDir, entry.name); + const children = new Set( + (await fs.readdir(dirPath, { withFileTypes: true })) + .filter(c => c.isFile()) + .map(c => c.name), + ); + const match = indexCandidates.find(c => children.has(c)); + if (match) entryFiles.push(path.join(dirPath, match)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 805 - 832, The function getComponentEntryFiles currently stats up to 4 candidate filenames per subdirectory and swallows all errors; replace that inner loop by performing a single await fs.readdir(path.join(sourceDir, entry.name)) to get the subdirectory listing, check that listing for any of the indexCandidates (index.tsx, index.jsx, index.ts, index.js) and when found push the joined candidate path to entryFiles (optionally verify it's a file with one fs.stat if needed). When calling fs.readdir for a subdirectory, only ignore ENOENT (directory missing) — rethrow other errors (e.g., EACCES) instead of silently swallowing them so permission issues surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 685-719: The union handling branch for ts.isUnionTypeNode filters
out null/undefined into actualTypes but doesn't handle the case when all members
were null/undefined, which leads to returning an empty oneOf; add a guard right
after computing actualTypes (check actualTypes.length === 0) and return a valid
schema (e.g., { type: 'null' }) so consumers don't receive { oneOf: [] }; update
the block around actualTypes, parseTypeNode and dedupeSchemas to place this
early-return before the later checks that build enums/oneOf.
---
Duplicate comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 516-533: collectInterfaceProperties currently only uses
declaration.members so inherited props from declaration.heritageClauses are
dropped; update collectInterfaceProperties to iterate
declaration.heritageClauses[*].types, resolve each type via
context.localDeclarations, and for each resolved declaration call
parseNamedDeclaration (respecting parseState.seen to avoid cycles) to obtain its
members, then merge those inherited members with the direct declaration.members
before calling collectTypeElementProperties; if a heritage type cannot be
resolved locally, throw a clear error naming the unresolved base type so callers
can add an `@a2uiSchema` override or flatten the declaration (this aligns with
buildObjectSchema's GENERIC_PROPS filtering expectations).
- Around line 115-121: The current blanket ts.isExportDeclaration check rejects
all export declarations including type-only re-exports; change it to only throw
for value re-exports by checking statement.moduleSpecifier and that the export
is not type-only. Concretely, update the condition to something like: if
(ts.isExportDeclaration(statement) && statement.moduleSpecifier &&
!statement.isTypeOnly) { throw new Error(...) } so that type-only exports
(statement.isTypeOnly) and local export clauses are allowed, while cross-module
value re-exports still produce the error referencing sourceFile.fileName and
statement.getText(sourceFile).
---
Nitpick comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 399-414: In buildObjectSchema: remove the unreachable else-if and
redundant allocations by returning directly for the topLevel case using the
constructed propertyMap and required array (i.e., return { properties:
propertyMap, required }) instead of relying on the pre-initialized schema
object; eliminate the useless alias additionalProperties and use
explicitAdditionalProperties directly; also drop the unreachable branch that
checks required.length > 0 after propertyNames is empty and simplify the logic
that assigns schema.required when propertyNames.length > 0 to assign required
onto schema (or the returned object) only when there are properties.
- Around line 805-832: The function getComponentEntryFiles currently stats up to
4 candidate filenames per subdirectory and swallows all errors; replace that
inner loop by performing a single await fs.readdir(path.join(sourceDir,
entry.name)) to get the subdirectory listing, check that listing for any of the
indexCandidates (index.tsx, index.jsx, index.ts, index.js) and when found push
the joined candidate path to entryFiles (optionally verify it's a file with one
fs.stat if needed). When calling fs.readdir for a subdirectory, only ignore
ENOENT (directory missing) — rethrow other errors (e.g., EACCES) instead of
silently swallowing them so permission issues surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92d2e2bd-0196-4511-8bba-cd381324c0a4
📒 Files selected for processing (4)
packages/genui/a2ui-catalog-extractor/rslib.config.tspackages/genui/a2ui-catalog-extractor/src/cli.tspackages/genui/a2ui-catalog-extractor/src/extractor.tspackages/genui/a2ui-catalog-extractor/src/types.ts
✅ Files skipped from review due to trivial changes (2)
- packages/genui/a2ui-catalog-extractor/rslib.config.ts
- packages/genui/a2ui-catalog-extractor/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/genui/a2ui-catalog-extractor/src/cli.ts
Comment on lines
+685
to
+719
| if (ts.isUnionTypeNode(typeNode)) { | ||
| const actualTypes = typeNode.types.filter(type => | ||
| !isNullLikeTypeNode(type) && !isUndefinedLikeTypeNode(type) | ||
| ); | ||
|
|
||
| if ( | ||
| actualTypes.length === 2 | ||
| && actualTypes.every(type => isBooleanLiteralTypeNode(type)) | ||
| ) { | ||
| return { type: 'boolean' }; | ||
| } | ||
|
|
||
| if (actualTypes.length === 1) { | ||
| return parseTypeNode(actualTypes[0]!, context, parseState); | ||
| } | ||
|
|
||
| if ( | ||
| actualTypes.length > 0 | ||
| && actualTypes.every(type => isStringLiteralTypeNode(type)) | ||
| ) { | ||
| return { | ||
| type: 'string', | ||
| enum: actualTypes.map(type => (type.literal as ts.StringLiteral).text), | ||
| }; | ||
| } | ||
|
|
||
| const schemas = dedupeSchemas( | ||
| actualTypes.map(type => parseTypeNode(type, context, parseState)), | ||
| ); | ||
| if (schemas.length === 1) { | ||
| return schemas[0]!; | ||
| } | ||
|
|
||
| return { oneOf: schemas }; | ||
| } |
Contributor
There was a problem hiding this comment.
Pure null | undefined union yields malformed { oneOf: [] }.
After filtering null/undefined at Line 686-688, if every union member was null/undefined the code falls through all branches and returns { oneOf: [] } — an invalid JSON Schema that consumers will silently accept. Unlikely in practice for component props, but cheap to guard:
🔧 Proposed guard
const schemas = dedupeSchemas(
actualTypes.map(type => parseTypeNode(type, context, parseState)),
);
if (schemas.length === 1) {
return schemas[0]!;
}
+ if (schemas.length === 0) {
+ throw new Error(
+ `Union "${typeNode.getText(context.sourceFile)}" in ${context.filePath} has no representable members.`,
+ );
+ }
return { oneOf: schemas };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 685 -
719, The union handling branch for ts.isUnionTypeNode filters out null/undefined
into actualTypes but doesn't handle the case when all members were
null/undefined, which leads to returning an empty oneOf; add a guard right after
computing actualTypes (check actualTypes.length === 0) and return a valid schema
(e.g., { type: 'null' }) so consumers don't receive { oneOf: [] }; update the
block around actualTypes, parseTypeNode and dedupeSchemas to place this
early-return before the later checks that build enums/oneOf.
Merging this PR will improve performance by 17.3%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
46.8 ms | 39.9 ms | +17.3% |
Comparing PupilTong:codex/a2ui-catalog-extractor (a5186af) with main (7e5f54c)2
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(27d5872) during the generation of this report, so 7e5f54c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
React Example#7637 Bundle Size — 225.38KiB (0%).a5186af(current) vs 7e5f54c main#7632(baseline) Bundle metrics
|
| Current #7637 |
Baseline #7632 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
0% |
35.31% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
179 |
179 |
| Duplicate Modules |
69 |
69 |
| Duplicate Code |
44.57% |
44.57% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7637 |
Baseline #7632 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
79.63KiB |
79.63KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-ext... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9210 Bundle Size — 900.02KiB (0%).a5186af(current) vs 7e5f54c main#9205(baseline) Bundle metrics
|
| Current #9210 |
Baseline #9205 |
|
|---|---|---|
| Initial JS |
44.46KiB |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
230(+0.88%) |
228 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.28%(-0.04%) |
27.29% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9210 |
Baseline #9205 |
|
|---|---|---|
| JS |
495.88KiB |
495.88KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-ext... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#769 Bundle Size — 196.54KiB (0%).a5186af(current) vs 7e5f54c main#764(baseline) Bundle metrics
|
| Current #769 |
Baseline #764 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
43.38% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
173 |
173 |
| Duplicate Modules |
66 |
66 |
| Duplicate Code |
44.08% |
44.08% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #769 |
Baseline #764 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
85.31KiB |
85.31KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-ext... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#754 Bundle Size — 680.27KiB (0%).a5186af(current) vs 7e5f54c main#749(baseline) Bundle metrics
|
| Current #754 |
Baseline #749 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
39.69% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-ext... Project dashboard
Generated by RelativeCI Documentation Report issue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 282-308: The GENERIC_PROPS filter in buildObjectSchema is applied
unconditionally and thus removes framework-injected names from nested object
shapes; change the loop so the check uses the topLevel flag (i.e., only skip
properties when topLevel is true and GENERIC_PROPS.has(property.name)), keeping
property processing identical otherwise (still building propertyMap, required,
etc.); update references to property.name, GENERIC_PROPS, and topLevel in the
for loop that iterates over properties to scope the filter to top-level
component schemas.
In `@packages/genui/a2ui/src/catalog/Button/index.tsx`:
- Around line 9-14: The TSDoc block currently above ButtonActionContextBinding
is describing ButtonProps and will be picked up for the wrong symbol; remove the
existing doc comment that precedes the ButtonActionContextBinding interface and
place that exact comment immediately above the exported ButtonProps interface
declaration so ButtonProps retains the correct summary; ensure the comment text
remains unchanged and that it is directly adjacent to the ButtonProps interface
(exported symbol name: ButtonProps) to be picked up by the TypeDoc extractor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 128a719d-b180-4412-a731-1f2cc71c1737
📒 Files selected for processing (11)
.github/a2ui-catalog-extractor.instructions.mdpackages/genui/a2ui-catalog-extractor/README.mdpackages/genui/a2ui-catalog-extractor/SKILL.mdpackages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.mdpackages/genui/a2ui-catalog-extractor/src/docs.tspackages/genui/a2ui-catalog-extractor/src/extractor.tspackages/genui/a2ui-catalog-extractor/src/types.tspackages/genui/a2ui-catalog-extractor/test/extract-catalog.test.tspackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/catalog/ActionButton/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/tsconfig.jsonpackages/genui/a2ui/src/catalog/Button/index.tsx
✅ Files skipped from review due to trivial changes (4)
- packages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.md
- packages/genui/a2ui-catalog-extractor/README.md
- packages/genui/a2ui-catalog-extractor/SKILL.md
- packages/genui/a2ui-catalog-extractor/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/a2ui-catalog-extractor.instructions.md
- packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts
Comment on lines
+282
to
+308
| function buildObjectSchema( | ||
| properties: PropertyDefinition[], | ||
| context: SourceContext, | ||
| parseState: ParseState, | ||
| topLevel: boolean, | ||
| typeDocRecord?: TypeDocRecord, | ||
| explicitAdditionalProperties?: JsonSchema | boolean, | ||
| ): JsonSchema | ComponentSchema { | ||
| const schema: JsonSchema = topLevel ? {} : { type: 'object' }; | ||
| const propertyMap: Record<string, JsonSchema> = {}; | ||
| const required: string[] = []; | ||
| const additionalProperties = explicitAdditionalProperties; | ||
|
|
||
| for (const property of properties) { | ||
| if (GENERIC_PROPS.has(property.name)) continue; | ||
|
|
||
| const doc = property.doc ?? typeDocRecord?.properties.get(property.name); | ||
| const propertySchema = applyPropertyDoc( | ||
| parseTypeNode(property.typeNode, context, parseState), | ||
| doc, | ||
| ); | ||
|
|
||
| propertyMap[property.name] = propertySchema; | ||
| if (!property.optional) { | ||
| required.push(property.name); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate GENERIC_PROPS definition and surrounding context to confirm which names are silently filtered.
rg -nP --type=ts -C3 '\bGENERIC_PROPS\b'Repository: lynx-family/lynx-stack
Length of output: 1969
🏁 Script executed:
#!/bin/bash
# Get complete GENERIC_PROPS definition
cat -n packages/genui/a2ui-catalog-extractor/src/types.ts | head -20Repository: lynx-family/lynx-stack
Length of output: 791
🏁 Script executed:
#!/bin/bash
# Find all call sites of buildObjectSchema with context
rg -n 'buildObjectSchema' packages/genui/a2ui-catalog-extractor/src/extractor.ts | head -20Repository: lynx-family/lynx-stack
Length of output: 325
🏁 Script executed:
#!/bin/bash
# Check context around each buildObjectSchema call site to see topLevel parameter values
for line in 397 479 506 654 892; do
echo "=== Call at line $line ==="
sed -n "$((line-2)),$((line+2))p" packages/genui/a2ui-catalog-extractor/src/extractor.ts
doneRepository: lynx-family/lynx-stack
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Get wider context around each buildObjectSchema call to see topLevel parameter
for line in 397 479 506 654 892; do
echo "=== Call at line $line ==="
sed -n "$((line-3)),$((line+5))p" packages/genui/a2ui-catalog-extractor/src/extractor.ts
echo ""
doneRepository: lynx-family/lynx-stack
Length of output: 1252
🏁 Script executed:
#!/bin/bash
# Get function context for each call - look for 'function' or similar keywords above each line
for line in 397 479 506 654 892; do
echo "=== Function containing call at line $line ==="
awk -v start=1 -v end=$line 'NR<=end {buffer[NR%50]=$0} END {for (i=start; i<=end; i++) print buffer[i%50]}' packages/genui/a2ui-catalog-extractor/src/extractor.ts | grep -E '(^function |^type |^interface |^export |^ return buildObjectSchema)' | tail -3
echo ""
doneRepository: lynx-family/lynx-stack
Length of output: 571
🏁 Script executed:
#!/bin/bash
# Better approach: find the function definition by searching backwards
sed -n '1,397p' packages/genui/a2ui-catalog-extractor/src/extractor.ts | tail -50 | head -40Repository: lynx-family/lynx-stack
Length of output: 1241
🏁 Script executed:
#!/bin/bash
# Find where parseSyntheticTypedef is called
rg -n 'parseSyntheticTypedef' packages/genui/a2ui-catalog-extractor/src/extractor.ts -B2 -A2Repository: lynx-family/lynx-stack
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Check what functions call buildObjectSchema at lines 479, 506
# First, let's find function definitions around those lines
sed -n '450,510p' packages/genui/a2ui-catalog-extractor/src/extractor.ts | head -60Repository: lynx-family/lynx-stack
Length of output: 1690
🏁 Script executed:
#!/bin/bash
# Find all calls to parseNamedDeclaration and their topLevel parameter
rg -n 'parseNamedDeclaration' packages/genui/a2ui-catalog-extractor/src/extractor.ts -B2 -A5Repository: lynx-family/lynx-stack
Length of output: 728
🏁 Script executed:
#!/bin/bash
# Check what's at line 654 - which case is it?
sed -n '620,670p' packages/genui/a2ui-catalog-extractor/src/extractor.tsRepository: lynx-family/lynx-stack
Length of output: 1293
Scope GENERIC_PROPS filter to the top-level component schema.
buildObjectSchema is invoked for both top-level component props and nested object type literals / referenced type aliases (e.g., line 654 explicitly passes topLevel: false). The unconditional if (GENERIC_PROPS.has(property.name)) continue; at line 296 therefore strips any of these framework-injected names (id, surface, setValue, sendAction, dataContextPath, __template, component) from any nested object, not just the top-level shape. This silently drops user-authored fields with those names, leaving no diagnostic.
Since the intent is to exclude framework-injected props from the component schema itself, gate the filter on topLevel:
for (const property of properties) {
- if (GENERIC_PROPS.has(property.name)) continue;
+ if (topLevel && GENERIC_PROPS.has(property.name)) continue;
const doc = property.doc ?? typeDocRecord?.properties.get(property.name);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function buildObjectSchema( | |
| properties: PropertyDefinition[], | |
| context: SourceContext, | |
| parseState: ParseState, | |
| topLevel: boolean, | |
| typeDocRecord?: TypeDocRecord, | |
| explicitAdditionalProperties?: JsonSchema | boolean, | |
| ): JsonSchema | ComponentSchema { | |
| const schema: JsonSchema = topLevel ? {} : { type: 'object' }; | |
| const propertyMap: Record<string, JsonSchema> = {}; | |
| const required: string[] = []; | |
| const additionalProperties = explicitAdditionalProperties; | |
| for (const property of properties) { | |
| if (GENERIC_PROPS.has(property.name)) continue; | |
| const doc = property.doc ?? typeDocRecord?.properties.get(property.name); | |
| const propertySchema = applyPropertyDoc( | |
| parseTypeNode(property.typeNode, context, parseState), | |
| doc, | |
| ); | |
| propertyMap[property.name] = propertySchema; | |
| if (!property.optional) { | |
| required.push(property.name); | |
| } | |
| } | |
| function buildObjectSchema( | |
| properties: PropertyDefinition[], | |
| context: SourceContext, | |
| parseState: ParseState, | |
| topLevel: boolean, | |
| typeDocRecord?: TypeDocRecord, | |
| explicitAdditionalProperties?: JsonSchema | boolean, | |
| ): JsonSchema | ComponentSchema { | |
| const schema: JsonSchema = topLevel ? {} : { type: 'object' }; | |
| const propertyMap: Record<string, JsonSchema> = {}; | |
| const required: string[] = []; | |
| const additionalProperties = explicitAdditionalProperties; | |
| for (const property of properties) { | |
| if (topLevel && GENERIC_PROPS.has(property.name)) continue; | |
| const doc = property.doc ?? typeDocRecord?.properties.get(property.name); | |
| const propertySchema = applyPropertyDoc( | |
| parseTypeNode(property.typeNode, context, parseState), | |
| doc, | |
| ); | |
| propertyMap[property.name] = propertySchema; | |
| if (!property.optional) { | |
| required.push(property.name); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 282 -
308, The GENERIC_PROPS filter in buildObjectSchema is applied unconditionally
and thus removes framework-injected names from nested object shapes; change the
loop so the check uses the topLevel flag (i.e., only skip properties when
topLevel is true and GENERIC_PROPS.has(property.name)), keeping property
processing identical otherwise (still building propertyMap, required, etc.);
update references to property.name, GENERIC_PROPS, and topLevel in the for loop
that iterates over properties to scope the filter to top-level component
schemas.
Comment on lines
+9
to
+14
| /** | ||
| * Props for the Button catalog component. | ||
| */ | ||
| export interface ButtonActionContextBinding { | ||
| path: string; | ||
| } |
Contributor
There was a problem hiding this comment.
Misplaced TSDoc: this describes ButtonProps, not ButtonActionContextBinding.
The /** Props for the Button catalog component. */ block is attached to ButtonActionContextBinding because it immediately precedes that interface. Given this PR wires up a TypeDoc-driven catalog extractor that consumes TSDoc on exported interfaces, this description will be picked up for the wrong symbol and ButtonProps will lose its intended summary. Move the block down to ButtonProps at line 32.
📝 Proposed fix
-/**
- * Props for the Button catalog component.
- */
-export interface ButtonActionContextBinding {
+export interface ButtonActionContextBinding {
path: string;
}
@@
-export interface ButtonProps extends GenericComponentProps {
+/**
+ * Props for the Button catalog component.
+ */
+export interface ButtonProps extends GenericComponentProps {
child: string;
variant?: 'primary' | 'borderless';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui/src/catalog/Button/index.tsx` around lines 9 - 14, The
TSDoc block currently above ButtonActionContextBinding is describing ButtonProps
and will be picked up for the wrong symbol; remove the existing doc comment that
precedes the ButtonActionContextBinding interface and place that exact comment
immediately above the exported ButtonProps interface declaration so ButtonProps
retains the correct summary; ensure the comment text remains unchanged and that
it is directly adjacent to the ButtonProps interface (exported symbol name:
ButtonProps) to be picked up by the TypeDoc extractor.
PupilTong
force-pushed
the
codex/a2ui-catalog-extractor
branch
from
aeb86cb to
35a990a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/genui/a2ui-catalog-extractor/src/extractor.ts (2)
999-1017:loadCatalogConfigcasts dynamic-import results without runtime validation.
await import(...)may resolve to a module whosedefault(or top-level namespace) isn't an object — e.g., someone exports an array or a primitive — but the return type is asserted asRecord<string, JsonValue>. Consider a lightweight runtime guard that rejects non-plain-object configs with a clear error message pointing atresolvedPath, so misconfigurations fail at load time instead of surfacing asundefinedreads deeper in the CLI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 999 - 1017, loadCatalogConfig currently casts dynamic-import results to Record<string, JsonValue> without validating that the imported module (module.default or module) is a plain object, which can hide misconfigurations; update loadCatalogConfig to perform a runtime guard after the import that checks whether the resolved value assigned to config is a non-null plain object (use resolvedPath in the error message) and reject (throw) with a clear error if it's not, so any array/primitive exports fail fast; reference loadCatalogConfig, resolvedPath and the local variable config when adding the validation and error throw.
310-340: Minor dead/redundant branches inbuildObjectSchema.
- Lines 311-315 set
schema.propertiesfor thetopLevelpath, but thetopLevelreturn at Line 317-322 immediately re-derives it viaschema.properties ?? {}, so that branch only matters for the non-top-level case.- At Line 328-332,
required.length > 0is only reachable whenpropertyNames.length > 0(required is pushed in the same loop that fillspropertyMap), so theelse ifis dead.Consider simplifying to keep the control flow easier to reason about.
Proposed cleanup
- const propertyNames = Object.keys(propertyMap); - if (propertyNames.length > 0) { - schema.properties = propertyMap; - } else if (topLevel) { - schema.properties = {}; - } - - if (topLevel) { + const propertyNames = Object.keys(propertyMap); + if (topLevel) { return { - properties: schema.properties ?? {}, + properties: propertyMap, required, }; } + if (propertyNames.length > 0) { + schema.properties = propertyMap; + } + if (propertyNames.length === 0 && additionalProperties === undefined) { return { type: 'object' }; } - if (propertyNames.length > 0) { - schema.required = required; - } else if (required.length > 0) { + if (propertyNames.length > 0) { schema.required = required; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts` around lines 310 - 340, In buildObjectSchema, remove the redundant topLevel branch that pre-sets schema.properties (the block that sets schema.properties when topLevel before the early return) and instead let the return use propertyMap directly; also remove the dead else-if that sets schema.required when propertyNames === 0 (since required is only populated when propertyNames > 0) and replace the required logic with a single conditional that assigns schema.required = required only when required.length > 0; finally collapse the additionalProperties logic to: if additionalProperties is undefined and propertyNames > 0 set schema.additionalProperties = false, otherwise set schema.additionalProperties = additionalProperties (so you only have one assignment path).packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts (1)
219-244: Golden-fixture test is brittle but intentional.The hard-coded
toHaveLength(10)and per-file equality againstlegacy-baseline/<Component>/catalog.jsonwill need updating whenever a new component is added or renamed inpackages/genui/a2ui/src/catalog. That's arguably the point of a golden test, but consider adding a reciprocal assertion that every fixture directory underlegacy-baselinecorresponds to a rendered file, so stale fixtures (e.g., after a component removal) don't silently stay green.Proposed extra assertion
expect(renderedFiles).toHaveLength(10); + + const fixtureDirs = (await fs.readdir(fixturePath('legacy-baseline'), { + withFileTypes: true, + })) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort(); + const renderedNames = renderedFiles + .map((file) => path.basename(path.dirname(file.path))) + .sort(); + expect(renderedNames).toEqual(fixtureDirs);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts` around lines 219 - 244, The golden-fixture test currently only asserts renderedFiles length and that each rendered file matches its fixture, which allows stale fixtures to persist; update the test (the 'matches the legacy A2UI catalog fixtures exactly' case) to also read the fixture directories under 'legacy-baseline' (use fixturePath helper to locate the base), derive each fixture's component name, and assert that a corresponding entry exists in renderedFiles (match by path/componentName) so every fixture directory has a rendered file; keep existing checks using extractCatalog, renderCatalogFiles, renderedFiles, and the per-file content equality assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/src/docs.ts`:
- Around line 254-277: The `@typedef/`@property parsing fails on nested brace type
expressions because the regex uses a greedy capture for {type}; replace the
fragile regex approach by extracting the braced type with a small parser:
implement a helper (e.g., extractBracedContent) that scans from the opening '{'
and returns the balanced content accounting for nested braces, then use that to
populate typedefTypeExpression/property type before matching the rest of the
line (use the existing typedefLine and lines loops and property parsing logic),
and additionally add a warning log when a line starts with '@property ' but no
property was extracted (so authors aren't silently losing fields); keep
JsDocTypedef shape and property accumulation unchanged.
---
Nitpick comments:
In `@packages/genui/a2ui-catalog-extractor/src/extractor.ts`:
- Around line 999-1017: loadCatalogConfig currently casts dynamic-import results
to Record<string, JsonValue> without validating that the imported module
(module.default or module) is a plain object, which can hide misconfigurations;
update loadCatalogConfig to perform a runtime guard after the import that checks
whether the resolved value assigned to config is a non-null plain object (use
resolvedPath in the error message) and reject (throw) with a clear error if it's
not, so any array/primitive exports fail fast; reference loadCatalogConfig,
resolvedPath and the local variable config when adding the validation and error
throw.
- Around line 310-340: In buildObjectSchema, remove the redundant topLevel
branch that pre-sets schema.properties (the block that sets schema.properties
when topLevel before the early return) and instead let the return use
propertyMap directly; also remove the dead else-if that sets schema.required
when propertyNames === 0 (since required is only populated when propertyNames >
0) and replace the required logic with a single conditional that assigns
schema.required = required only when required.length > 0; finally collapse the
additionalProperties logic to: if additionalProperties is undefined and
propertyNames > 0 set schema.additionalProperties = false, otherwise set
schema.additionalProperties = additionalProperties (so you only have one
assignment path).
In `@packages/genui/a2ui-catalog-extractor/test/extract-catalog.test.ts`:
- Around line 219-244: The golden-fixture test currently only asserts
renderedFiles length and that each rendered file matches its fixture, which
allows stale fixtures to persist; update the test (the 'matches the legacy A2UI
catalog fixtures exactly' case) to also read the fixture directories under
'legacy-baseline' (use fixturePath helper to locate the base), derive each
fixture's component name, and assert that a corresponding entry exists in
renderedFiles (match by path/componentName) so every fixture directory has a
rendered file; keep existing checks using extractCatalog, renderCatalogFiles,
renderedFiles, and the per-file content equality assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2187b9b3-d280-41d8-9667-ee5be1ccc07e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (63)
.github/a2ui-catalog-extractor.instructions.mdpackages/genui/a2ui-catalog-extractor/README.mdpackages/genui/a2ui-catalog-extractor/SKILL.mdpackages/genui/a2ui-catalog-extractor/agents/openai.yamlpackages/genui/a2ui-catalog-extractor/eslint.config.jspackages/genui/a2ui-catalog-extractor/package.jsonpackages/genui/a2ui-catalog-extractor/references/a2ui-v0.9-schema.mdpackages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.mdpackages/genui/a2ui-catalog-extractor/rslib.config.tspackages/genui/a2ui-catalog-extractor/rstest.config.tspackages/genui/a2ui-catalog-extractor/src/cli.tspackages/genui/a2ui-catalog-extractor/src/docs.tspackages/genui/a2ui-catalog-extractor/src/extractor.tspackages/genui/a2ui-catalog-extractor/src/index.tspackages/genui/a2ui-catalog-extractor/src/types.tspackages/genui/a2ui-catalog-extractor/test/cli.test.tspackages/genui/a2ui-catalog-extractor/test/extract-catalog.test.tspackages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/catalog/Loose/index.jsxpackages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/jsx/catalog/Badge/index.jsxpackages/genui/a2ui-catalog-extractor/test/fixtures/jsx/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Button/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Card/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/CheckBox/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Column/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Divider/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Image/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/List/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/RadioGroup/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Row/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Text/catalog.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/catalog/ActionButton/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/catalog/UndefinedDefault/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/catalog/DefaultExport/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/catalog/NamedExport/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/catalog/BrokenJson/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/catalog/Fancy/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx/catalog/Chip/index.tsxpackages/genui/a2ui-catalog-extractor/test/fixtures/tsx/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/tsconfig.build.jsonpackages/genui/a2ui-catalog-extractor/tsconfig.jsonpackages/genui/a2ui-catalog-extractor/turbo.jsonpackages/genui/a2ui/README.mdpackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Card/index.tsxpackages/genui/a2ui/src/catalog/CheckBox/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/catalog/Divider/index.tsxpackages/genui/a2ui/src/catalog/Image/index.tsxpackages/genui/a2ui/src/catalog/List/index.tsxpackages/genui/a2ui/src/catalog/RadioGroup/index.tsxpackages/genui/a2ui/src/catalog/Row/index.tsxpackages/genui/a2ui/src/catalog/Text/index.tsxpackages/genui/a2ui/tools/catalog_generator.tspackages/genui/a2ui/turbo.jsonpackages/genui/tsconfig.json
💤 Files with no reviewable changes (1)
- packages/genui/a2ui/tools/catalog_generator.ts
✅ Files skipped from review due to trivial changes (46)
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Card/catalog.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/catalog/Loose/index.jsx
- packages/genui/a2ui/src/catalog/Divider/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Divider/catalog.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/tsconfig.json
- packages/genui/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/catalog/BrokenJson/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-default/catalog/UndefinedDefault/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-json/tsconfig.json
- packages/genui/a2ui/src/catalog/Text/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/List/catalog.json
- packages/genui/a2ui-catalog-extractor/tsconfig.build.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx/tsconfig.json
- packages/genui/a2ui-catalog-extractor/tsconfig.json
- packages/genui/a2ui/src/catalog/CheckBox/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/RadioGroup/catalog.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-named/catalog/NamedExport/index.tsx
- packages/genui/a2ui/src/catalog/Image/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx-loose/tsconfig.json
- packages/genui/a2ui/src/catalog/RadioGroup/index.tsx
- packages/genui/a2ui/turbo.json
- packages/genui/a2ui-catalog-extractor/rslib.config.ts
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Row/catalog.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/catalog/DefaultExport/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Image/catalog.json
- packages/genui/a2ui-catalog-extractor/agents/openai.yaml
- .github/a2ui-catalog-extractor.instructions.md
- packages/genui/a2ui-catalog-extractor/turbo.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/CheckBox/catalog.json
- packages/genui/a2ui/src/catalog/List/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-type/catalog/Fancy/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx/catalog/Chip/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx/catalog/Badge/index.jsx
- packages/genui/a2ui-catalog-extractor/src/index.ts
- packages/genui/a2ui/README.md
- packages/genui/a2ui/src/catalog/Card/index.tsx
- packages/genui/a2ui-catalog-extractor/README.md
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Text/catalog.json
- packages/genui/a2ui-catalog-extractor/SKILL.md
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-invalid-export-default/tsconfig.json
- packages/genui/a2ui-catalog-extractor/src/types.ts
- packages/genui/a2ui/src/catalog/Column/index.tsx
- packages/genui/a2ui-catalog-extractor/test/fixtures/tsx-complex/catalog/ActionButton/index.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/genui/a2ui-catalog-extractor/rstest.config.ts
- packages/genui/a2ui-catalog-extractor/test/fixtures/jsx/tsconfig.json
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Column/catalog.json
- packages/genui/a2ui-catalog-extractor/package.json
- packages/genui/a2ui-catalog-extractor/references/tsdoc-mapping.md
- packages/genui/a2ui-catalog-extractor/test/fixtures/legacy-baseline/Button/catalog.json
- packages/genui/a2ui/package.json
- packages/genui/a2ui-catalog-extractor/src/cli.ts
- packages/genui/a2ui/src/catalog/Button/index.tsx
- packages/genui/a2ui-catalog-extractor/test/cli.test.ts
Comment on lines
+254
to
+277
| const typedefMatch = typedefLine.match( | ||
| /^@typedef\s+\{(?<type>.+)\}\s+(?<name>[A-Za-z_$][\w$]*)(?:\s+(?<description>.+))?$/u, | ||
| ); | ||
| if (!typedefMatch?.groups) continue; | ||
|
|
||
| const typedef: JsDocTypedef = { | ||
| name: typedefMatch.groups['name']!, | ||
| properties: [], | ||
| }; | ||
| const typedefDescription = typedefMatch.groups['description']?.trim(); | ||
| const typedefTypeExpression = typedefMatch.groups['type']?.trim(); | ||
| if (typedefDescription) { | ||
| typedef.description = typedefDescription; | ||
| } | ||
| if (typedefTypeExpression) { | ||
| typedef.typeExpression = typedefTypeExpression; | ||
| } | ||
|
|
||
| for (const line of lines) { | ||
| if (!line.startsWith('@property ')) continue; | ||
| const propertyMatch = line.match( | ||
| /^@property\s+\{(?<type>.+)\}\s+(?<name>\[[^\]]+\]|[A-Za-z_$][\w$]*)(?:\s+(?<description>.+))?$/u, | ||
| ); | ||
| if (!propertyMatch?.groups) continue; |
Contributor
There was a problem hiding this comment.
@typedef / @property regexes don't tolerate nested braces in the type expression.
\{(?<type>.+)\} matches greedily up to the last } on the line, so it silently breaks on JSDoc types that embed braces, e.g. @property {{ path: string }} label or @property {Record<string, {a: number}>} ctx — the entire typedef/property line will fail to match and be dropped without diagnostic. For JSX best-effort mode that's a realistic authoring shape.
If supporting nested braces is out of scope, consider at least logging/throwing when an @property line fails to match, so authors don't silently lose fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/genui/a2ui-catalog-extractor/src/docs.ts` around lines 254 - 277,
The `@typedef/`@property parsing fails on nested brace type expressions because
the regex uses a greedy capture for {type}; replace the fragile regex approach
by extracting the braced type with a small parser: implement a helper (e.g.,
extractBracedContent) that scans from the opening '{' and returns the balanced
content accounting for nested braces, then use that to populate
typedefTypeExpression/property type before matching the rest of the line (use
the existing typedefLine and lines loops and property parsing logic), and
additionally add a warning log when a line starts with '@property ' but no
property was extracted (so authors aren't silently losing fields); keep
JsDocTypedef shape and property accumulation unchanged.
5aa880b to
9a62559
Compare
PupilTong
force-pushed
the
codex/a2ui-catalog-extractor
branch
from
9a62559 to
a5186af
Compare
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
1 participant
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.
Summary by CodeRabbit
New Features
Documentation
Chores
Bug Fixes / Minor API
Summary
@lynx-js/a2ui-catalog-extractorworkspace package that can be used as an API, CLI, and installable Codex skillTesting
Validation
fnm exec --using v24.15.0 -- pnpm --filter @lynx-js/a2ui-catalog-extractor buildfnm exec --using v24.15.0 -- pnpm --filter @lynx-js/a2ui-catalog-extractor testfnm exec --using v24.15.0 -- pnpm --filter @lynx-js/a2ui-reactlynx buildNotes
.tsxas the primary authoring path, supports.jsxas a best-effort path, and uses@a2uiSchemaas the only custom escape hatch