cycle/0012: outline quality audit#7
Merged
Conversation
Real-world code patterns: React components with hooks/JSX, Express router with 15 arrow handlers, barrel re-exports, god class with 30+ methods (static/get/set/async/private), mixed declarations (interfaces/types/classes/enums/functions), dense generics with long signatures, NestJS-style decorators. Step 1/6 of CORE_outline-quality-audit (cycle 0012).
40 tests across 7 fixture categories. 34 pass with current parser, 6 skipped pending parser fixes (arrow functions, enums, re-exports). Findings: JSX, complex generics, and decorators trigger partial parse but symbols still extract correctly via error recovery. Step 2/6 of CORE_outline-quality-audit (cycle 0012).
Arrow functions assigned to exported constants (export const fn = () => {})
now get kind: "function" with extracted parameter/return type signatures
instead of the generic kind: "export" with no signature.
This is the highest-impact parser fix — arrow functions are the dominant
pattern in modern TS/React codebases. Agents can now distinguish a
50-line arrow function from a config constant in the outline.
Step 3/6 of CORE_outline-quality-audit (cycle 0012).
Add "enum" to OutlineEntry.kind union. Map enum_declaration in nodeKind() and the export_statement handler. Enums now appear in outlines and jump tables. Step 4/6 of CORE_outline-quality-audit (cycle 0012).
Named re-exports (export { A, B } from './x') now appear as individual
outline entries. Type re-exports (export type { T } from './x') are
also captured. Wildcard re-exports (export * from './x') appear as a
single entry. Barrel files are no longer invisible to agents.
Step 5/6 of CORE_outline-quality-audit (cycle 0012).
Delete CORE_outline-quality-audit ASAP ticket. Add retro documenting 7 fixtures, 40 assertions, 3 parser fixes. Update CHANGELOG with Added section for arrow functions, enums, and re-exports. 267 tests (40 new), zero skips.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR enhances the outline parser to support enum declarations, emit properly-typed signatures for arrow function exports, and detect re-export forms (named, type, and wildcard), preventing barrel files from being omitted. A comprehensive audit cycle with seven fixture files and forty test assertions validates the extraction completeness. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b85c70e7ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+286
to
+289
| const exportClause = child.namedChildren.find( | ||
| (c) => c.type === "export_clause", | ||
| ); | ||
| if (exportClause) { |
There was a problem hiding this comment.
This export_clause branch runs for both re-exports and local export lists, so a module like function foo() {}; export { foo }; now produces two outline entries for foo (the original function plus a synthetic kind: "export" entry). That duplicates symbols and can downgrade navigation quality because one entry has no signature and the function entry still appears as exported: false. Restrict this path to clauses that actually have a from source (true re-exports), or merge with an existing symbol instead of appending a second entry.
Useful? React with 👍 / 👎.
Comment on lines
+304
to
+307
| const hasWildcard = child.children.some((c) => c.type === "*" || c.type === "namespace_export"); | ||
| if (hasWildcard && source) { | ||
| const name = `* from ${source.text}`; | ||
| entries.push({ kind: "export", name, exported: true }); |
There was a problem hiding this comment.
export * as re-exports
For namespace re-exports (e.g. export * as ns from "./mod"), this code emits the synthetic name * from "./mod" instead of the actual exported symbol ns. That makes symbol lookup/jump-table usage by exported name fail for this syntax. The wildcard branch should read the alias from namespace_export when present and only use a * from ... label for bare export * from ....
Useful? React with 👍 / 👎.
Rename local 'source' to 'moduleSpecifier' in the wildcard re-export handler to avoid shadowing the outer 'source' parameter. Add tests for wildcard re-export and non-function const export (VERSION).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/parser/outline.ts`:
- Line 303: The inner const named "source" in the child.namedChildren.find(...)
shadows the outer parameter source of extractOutline(source: string, ...);
rename that inner variable to something unambiguous (e.g., sourceStringNode or
fromClause) and update all local references in the same block to use the new
name so you remove the shadowing without changing behavior; ensure you only
change the identifier in the scope of the child processing (the find call and
any subsequent uses) and leave the outer parameter name intact.
- Around line 170-184: The logic that builds the textual signature is duplicated
between extractFunctionSignature and extractArrowSignature; refactor by creating
a small helper (e.g., buildSignature(name: string, node: TSNode)) that reads
node.childForFieldName("parameters") and node.childForFieldName("return_type"),
composes the same param text and cleaned return type, and returns
boundSignature(result); then have extractFunctionSignature and
extractArrowSignature call this helper (pass the node and either the extracted
name or the name parameter) so both functions reuse the same composition and
boundSignature call.
- Around line 259-261: Replace the hard-coded repeated checks for declaration
node types with a single shared constant: create a file-scoped constant (e.g.
EXPORTED_DECLARATION_KINDS as a Set or array) listing the six node.type strings,
then use that constant in the filter currently checking c.type === ... and in
the nodeKind() function to determine exported declaration support; update both
usages (the c.type filter and nodeKind()) to test membership against
EXPORTED_DECLARATION_KINDS so future additions only require changing one
constant.
- Around line 285-310: The export_clause handling currently treats all
re-exports as kind: "export"; update the loop that processes export_specifier
nodes (in the exportClause branch) to detect if the clause is a type-only
re-export by checking for a child whose type/text is "type" (similar to the
hasWildcard pattern) and set entries.push(... { kind: "type", ... }) and
buildJumpEntry(..., "type", ...) when present; leave value re-exports as kind:
"export". Also update the related test assertions (the test around lines
128–133) to assert the kind field (expecting "type" for type re-exports) rather
than only toBeDefined().
In `@test/fixtures/audit/re-exports.ts`:
- Line 4: The barrel re-exports the default-exported component as a named
export: replace the incorrect named re-export of UserCard with a
default-to-named re-export that respects the module shape from
"./react-component" (export the module default as UserCard) while keeping
UserList as the named re-export; update the export statement in re-exports.ts to
re-export the module default as UserCard and also export UserList by name so the
AST reflects the actual exports of react-component.tsx.
In `@test/unit/parser/outline-audit.test.ts`:
- Around line 188-225: Several assertions access properties on byName(result,
"...") without first asserting the returned value exists; update each test that
uses byName (e.g., the checks for "Config", "Logger", "Status", "Result",
"Server", "createServer", "createClient", "parseConfig", "internalHelper",
"InternalCache", "LogLevel", "HttpMethod") to first assert the lookup is defined
(use expect(...).toBeDefined()) before accessing .kind, .exported, .children,
etc., mirroring the pattern already used in the earlier tests so failures
produce clear diagnostics.
- Around line 79-86: The tests "extracts both middleware functions as
kind=function" and "extracts RouteParams interface and HandlerFn type" access
byName(result, ...)!.kind without asserting the entry exists; add explicit
existence assertions (e.g., expect(byName(result,
"authenticate")).toBeDefined(), expect(byName(result,
"rateLimit")).toBeDefined(), expect(byName(result,
"RouteParams")).toBeDefined(), expect(byName(result,
"HandlerFn")).toBeDefined()) before reading .kind so failures produce a clear
"expected to be defined" message and avoid TypeError when byName returns
undefined.
- Around line 117-134: The "extracts named re-exports" test currently only
checks presence of UserCard and authenticate; update that test to assert their
kind is "export" by using the byName(result, "<symbol>") lookup (e.g., for
UserCard and authenticate) and adding assertions that their .kind equals
"export" so re-exports can't be misclassified as "function" or other kinds;
modify the same test case that references result and byName to include these
kind checks.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74c25148-2e34-41f1-ab1b-cd341ff367d6
📒 Files selected for processing (13)
CHANGELOG.mddocs/method/backlog/asap/CORE_outline-quality-audit.mddocs/method/retro/0012-outline-quality-audit/retro.mdsrc/parser/outline.tssrc/parser/types.tstest/fixtures/audit/decorated-class.tstest/fixtures/audit/dense-code.tstest/fixtures/audit/express-router.tstest/fixtures/audit/god-class.tstest/fixtures/audit/mixed-declarations.tstest/fixtures/audit/re-exports.tstest/fixtures/audit/react-component.tsxtest/unit/parser/outline-audit.test.ts
💤 Files with no reviewable changes (1)
- docs/method/backlog/asap/CORE_outline-quality-audit.md
Comment on lines
+170
to
+184
| function extractArrowSignature(name: string, arrowNode: TSNode): string | undefined { | ||
| const params = arrowNode.childForFieldName("parameters"); | ||
| const returnType = arrowNode.childForFieldName("return_type"); | ||
|
|
||
| let sig = name; | ||
| if (params) { | ||
| sig += params.text; | ||
| } | ||
| if (returnType) { | ||
| const rt = returnType.text.replace(/^:\s*/, ""); | ||
| sig += ": " + rt; | ||
| } | ||
|
|
||
| return boundSignature(sig); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
DRY violation: extractArrowSignature duplicates logic from extractFunctionSignature.
Lines 171-181 are nearly identical to lines 89-100. The only difference is that extractFunctionSignature extracts the name from the node, while extractArrowSignature takes it as a parameter. This is begging to diverge silently when one gets updated and the other doesn't.
♻️ Proposed refactor to consolidate signature extraction
-function extractFunctionSignature(node: TSNode): string | undefined {
- const nameNode = node.childForFieldName("name");
- if (!nameNode) return undefined;
-
- const params = node.childForFieldName("parameters");
- const returnType = node.childForFieldName("return_type");
-
- let sig = nameNode.text;
- if (params) {
- sig += params.text;
- }
- if (returnType) {
- // return_type node text includes the leading `: ` in some grammars
- const rt = returnType.text.replace(/^:\s*/, "");
- sig += ": " + rt;
- }
-
- return boundSignature(sig);
-}
+function buildSignature(name: string, node: TSNode): string {
+ const params = node.childForFieldName("parameters");
+ const returnType = node.childForFieldName("return_type");
+
+ let sig = name;
+ if (params) {
+ sig += params.text;
+ }
+ if (returnType) {
+ const rt = returnType.text.replace(/^:\s*/, "");
+ sig += ": " + rt;
+ }
+
+ return boundSignature(sig);
+}
+
+function extractFunctionSignature(node: TSNode): string | undefined {
+ const nameNode = node.childForFieldName("name");
+ if (!nameNode) return undefined;
+ return buildSignature(nameNode.text, node);
+}
-function extractArrowSignature(name: string, arrowNode: TSNode): string | undefined {
- const params = arrowNode.childForFieldName("parameters");
- const returnType = arrowNode.childForFieldName("return_type");
-
- let sig = name;
- if (params) {
- sig += params.text;
- }
- if (returnType) {
- const rt = returnType.text.replace(/^:\s*/, "");
- sig += ": " + rt;
- }
-
- return boundSignature(sig);
-}
+function extractArrowSignature(name: string, arrowNode: TSNode): string {
+ return buildSignature(name, arrowNode);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/parser/outline.ts` around lines 170 - 184, The logic that builds the
textual signature is duplicated between extractFunctionSignature and
extractArrowSignature; refactor by creating a small helper (e.g.,
buildSignature(name: string, node: TSNode)) that reads
node.childForFieldName("parameters") and node.childForFieldName("return_type"),
composes the same param text and cleaned return type, and returns
boundSignature(result); then have extractFunctionSignature and
extractArrowSignature call this helper (pass the node and either the extracted
name or the name parameter) so both functions reuse the same composition and
boundSignature call.
Comment on lines
+259
to
261
| c.type === "type_alias_declaration" || | ||
| c.type === "enum_declaration", | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Enumerate all supported exported declarations explicitly.
The list grew to 6 node types. Consider extracting this to a constant or set for maintainability - right now it's duplicated conceptually in nodeKind() (lines 61-79) and here. When the next declaration type comes along, someone will update one and forget the other.
♻️ Proposed refactor
+const EXPORTED_DECLARATION_TYPES = new Set([
+ "function_declaration",
+ "generator_function_declaration",
+ "class_declaration",
+ "interface_declaration",
+ "type_alias_declaration",
+ "enum_declaration",
+]);
// In extractOutline:
const inner = child.namedChildren.find(
- (c) =>
- c.type === "function_declaration" ||
- c.type === "generator_function_declaration" ||
- c.type === "class_declaration" ||
- c.type === "interface_declaration" ||
- c.type === "type_alias_declaration" ||
- c.type === "enum_declaration",
+ (c) => EXPORTED_DECLARATION_TYPES.has(c.type),
);📝 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
| c.type === "type_alias_declaration" || | |
| c.type === "enum_declaration", | |
| ); | |
| const EXPORTED_DECLARATION_TYPES = new Set([ | |
| "function_declaration", | |
| "generator_function_declaration", | |
| "class_declaration", | |
| "interface_declaration", | |
| "type_alias_declaration", | |
| "enum_declaration", | |
| ]); | |
| // ... (elsewhere in the file, in extractOutline function around lines 259-261) | |
| const inner = child.namedChildren.find( | |
| (c) => EXPORTED_DECLARATION_TYPES.has(c.type), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/parser/outline.ts` around lines 259 - 261, Replace the hard-coded
repeated checks for declaration node types with a single shared constant: create
a file-scoped constant (e.g. EXPORTED_DECLARATION_KINDS as a Set or array)
listing the six node.type strings, then use that constant in the filter
currently checking c.type === ... and in the nodeKind() function to determine
exported declaration support; update both usages (the c.type filter and
nodeKind()) to test membership against EXPORTED_DECLARATION_KINDS so future
additions only require changing one constant.
Comment on lines
+285
to
+310
| // Re-exports: export { A, B } from './x' | ||
| const exportClause = child.namedChildren.find( | ||
| (c) => c.type === "export_clause", | ||
| ); | ||
| if (exportClause) { | ||
| for (const spec of exportClause.namedChildren) { | ||
| if (spec.type === "export_specifier") { | ||
| const nameNode = spec.childForFieldName("alias") ?? spec.childForFieldName("name"); | ||
| if (nameNode) { | ||
| entries.push({ kind: "export", name: nameNode.text, exported: true }); | ||
| jumpTable.push(buildJumpEntry(nameNode.text, "export", child)); | ||
| } | ||
| } | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // Wildcard re-export: export * from './x' | ||
| const source = child.namedChildren.find((c) => c.type === "string"); | ||
| const hasWildcard = child.children.some((c) => c.type === "*" || c.type === "namespace_export"); | ||
| if (hasWildcard && source) { | ||
| const name = `* from ${source.text}`; | ||
| entries.push({ kind: "export", name, exported: true }); | ||
| jumpTable.push(buildJumpEntry(name, "export", child)); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that type-only re-exports are captured and check the AST structure
# First, find the re-exports fixture
cat test/fixtures/audit/re-exports.ts
echo "---"
# Check the test assertions for type re-exports
rg -n "type.*re-?export" test/unit/parser/outline-audit.test.tsRepository: flyingrobots/graft
Length of output: 662
🏁 Script executed:
# Get the full test for type re-exports
sed -n '130,160p' test/unit/parser/outline-audit.test.ts
echo "---"
# Check the full context around lines 285-310 in outline.ts
sed -n '270,320p' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 3050
🏁 Script executed:
# Check if there's special handling for "export_type" or type exports
rg -n "export_type|type.*export" src/parser/outline.ts
echo "---"
# Look for how the "kind" field is determined for exports
rg -n "kind.*export|export.*kind" src/parser/outline.ts | head -20
echo "---"
# Check the full export handling block and context above line 285
sed -n '250,340p' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 3418
🏁 Script executed:
# Check if there's handling for export_type_statement
rg -n "export_type_statement" src/
echo "---"
# Look for all node type checks in outline.ts to see all export types handled
rg -n 'child\.type === "export' src/parser/outline.ts
echo "---"
# Check what tree-sitter node types are possible by looking at grammar or parser reference
find . -name "*.md" -o -name "grammar*" | head -5Repository: flyingrobots/graft
Length of output: 187
🏁 Script executed:
# Get full test context including expectations on kind
sed -n '128,145p' test/unit/parser/outline-audit.test.ts
echo "---"
# Check CLAUDE.md for guidance on tree-sitter usage
grep -A 5 -B 5 "tree-sitter\|export\|type" CLAUDE.md | head -40
echo "---"
# Write a script to parse an export type statement and inspect its AST
cat > /tmp/test_ast.mjs << 'EOF'
import Parser from 'web-tree-sitter';
await Parser.init();
const parser = new Parser();
// Try to find tree-sitter-javascript
try {
const JavaScript = await Parser.Language.load('/tmp/tree-sitter-javascript.wasm');
parser.setLanguage(JavaScript);
const code = `export type { UserProps } from './x';`;
const tree = parser.parse(code);
function printTree(node, indent = 0) {
console.log(' '.repeat(indent) + `${node.type}${node.isNamed ? '' : ' (anon)'}`);
for (let child of node.namedChildren) {
printTree(child, indent + 2);
}
}
printTree(tree.rootNode);
} catch(e) {
console.log("Could not load tree-sitter WASM:", e.message);
}
EOFRepository: flyingrobots/graft
Length of output: 1777
🏁 Script executed:
# Search for "type" keyword handling in export context
rg -B3 -A3 "export.*type|type.*export" src/parser/outline.ts
echo "---"
# Check if there's a check for child nodes containing "type" keyword
rg -n "childForFieldName|type.*keyword|keyword.*type" src/parser/outline.ts | head -20
echo "---"
# Look at the test to see if it actually verifies the kind field
grep -A 10 "extracts type re-exports" test/unit/parser/outline-audit.test.tsRepository: flyingrobots/graft
Length of output: 2242
🏁 Script executed:
# Search for how the code should handle type vs value exports
# Check if there are any comments or TODOs about this
rg -B2 -A2 "TODO|FIXME|type.*export|export.*type" src/parser/outline.ts
echo "---"
# Check what the complete test file expects for kind on type exports
grep -B5 -A15 'extracts type re-exports' test/unit/parser/outline-audit.test.ts
echo "---"
# Look for other places where "type" kind is assigned to see the pattern
rg -n 'kind.*:.*"type"|kind:.*"type"' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 1737
🏁 Script executed:
# Check package.json for tree-sitter dependencies
cat package.json | grep -A5 -B5 "tree-sitter\|@babel"
echo "---"
# Check if there's any configuration or initialization of tree-sitter
rg -n "Parser\|Language\.load\|setLanguage" src/parser/outline.ts | head -20
echo "---"
# Look for handling of the "type" keyword in export statements
# Check all children iteration to see if there's type keyword checking
sed -n '280,315p' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 1909
🏁 Script executed:
# Look at the OutlineEntry type definition to see what kind values are valid
rg -B5 -A10 "interface OutlineEntry|type OutlineEntry" src/parser/outline.ts
echo "---"
# Check what kind values are actually used in the codebase
rg -o 'kind:\s*"[^"]*"' src/parser/outline.ts | sort -u
echo "---"
# Check if there's a nodeKind function that maps types
sed -n '/^function nodeKind/,/^}/p' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 614
🏁 Script executed:
# Search for any keyword checking in the codebase
rg -n 'type === "keyword"' src/parser/outline.ts
echo "---"
# Look for how other parts distinguish between different kinds of statements
# Check the full export_statement handling block
sed -n '245,320p' src/parser/outline.ts
echo "---"
# Check tree-sitter-wasms package to see available grammar versions
cat package-lock.json | grep -A 10 "tree-sitter-wasms"Repository: flyingrobots/graft
Length of output: 2827
🏁 Script executed:
# Look for any test or fixture that might show the AST structure
find test -name "*.ts" -o -name "*.js" | xargs grep -l "export type" | head -3
echo "---"
# Check if there are any utilities that parse and print AST
rg -l "printTree\|debugTree\|astNodeInfo" src/ test/
echo "---"
# Look at what children the export_statement has - check if "type" keyword is a child
# Check all children iteration patterns
rg -n "\.children\|\.namedChildren" src/parser/outline.ts | grep -A2 -B2 "export"
echo "---"
# Check if there's a "type" keyword child in export_statement
# by looking at what types are checked
sed -n '250,330p' src/parser/outline.ts | grep "c\.type\|type ==="Repository: flyingrobots/graft
Length of output: 838
🏁 Script executed:
# Create a minimal test to understand tree-sitter's AST for export type
cat > /tmp/check_ast.js << 'EOF'
const fs = require('fs');
const path = require('path');
// Check what tree-sitter package provides
try {
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
const deps = pkg.dependencies || {};
console.log('tree-sitter version:', deps['web-tree-sitter']);
console.log('tree-sitter-wasms version:', deps['tree-sitter-wasms']);
} catch(e) {
console.log('Error:', e.message);
}
EOF
node /tmp/check_ast.js
echo "---"
# Check the re-exports.ts fixture more carefully to understand the test
cat test/fixtures/audit/re-exports.ts
echo "---"
# Check if there's explicit handling for "type" keyword anywhere
rg -n '"type"' src/parser/outline.ts | head -30Repository: flyingrobots/graft
Length of output: 708
🏁 Script executed:
# Look for how the code checks for keyword nodes
# The hasWildcard check shows it iterates child.children for "*"
# Check if we should do the same for "type"
rg -B3 -A3 'child\.children\.some' src/parser/outline.ts
echo "---"
# Check if there's a "type" keyword check anywhere in the export handling
# by examining all checks in the export_statement block
sed -n '251,315p' src/parser/outline.ts | grep -E "child\.|\.some|\.find"
echo "---"
# Search for what tree-sitter node type represents the "type" keyword in exports
# by checking tree-sitter documentation or comments
grep -r "type.*keyword\|keyword.*type" test/ src/ 2>/dev/null | head -5Repository: flyingrobots/graft
Length of output: 853
🏁 Script executed:
# Check if there's a pattern of checking for keyword nodes
# Look at how other similar checks are done
rg -B2 -A2 'type === "keyword"' src/
echo "---"
# See if the code checks for "type" keyword anywhere in statement handling
rg -n 'keyword\|"type"\s*===|=== *"type"' src/parser/outline.ts | head -20
echo "---"
# Most importantly, let's check what the test actually validates
# Get the complete test including all assertions for type re-exports
sed -n '125,150p' test/unit/parser/outline-audit.test.ts
echo "---"
# Check if kind field is ever validated in tests
grep -n "\.kind\|expect.*kind" test/unit/parser/outline-audit.test.ts | head -15Repository: flyingrobots/graft
Length of output: 1893
🏁 Script executed:
# Check if the re-exports test actually validates kind for type re-exports
# Looking for the actual line numbers and assertions
sed -n '128,140p' test/unit/parser/outline-audit.test.ts
echo "---"
# The issue might be that the test checks lines 85-86 which validate kind
# But those might not be from re-exports, let's check what fixture they're from
sed -n '70,90p' test/unit/parser/outline-audit.test.ts
echo "---"
# Verify: do type re-exports need different handling?
# Check if there's a distinction needed between export { } and export type { }
grep -n "export_type" src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 1312
🏁 Script executed:
# Check the express-router fixture to see if RouteParams and HandlerFn are declared there
cat test/fixtures/audit/express-router.ts
echo "---"
# Check react-component fixture
cat test/fixtures/audit/react-component.ts
echo "---"
# Now let's understand: when we export type { Foo } from '...' is the kind supposed to be "type" or "export"?
# Check if there's any distinction in the codebase between type and value re-exports
rg -n "kind.*type.*export|export.*kind.*type" src/ test/Repository: flyingrobots/graft
Length of output: 3074
🏁 Script executed:
# The key insight: when you export type { RouteParams } from './express-router',
# RouteParams was originally declared as 'export interface RouteParams'
# The question is: should the re-export preserve the original kind "interface"
# or should it be marked as "export"?
# Let's check what the test actually expects by looking at re-exports test more carefully
sed -n '119,145p' test/unit/parser/outline-audit.test.ts
echo "---"
# Check if there's any lookup of original kind for re-exports
rg -B5 -A10 "export.*kind|kind.*export" src/parser/outline.ts | head -40
echo "---"
# The core issue: does code distinguish between:
# - export { named } from './x' (re-export)
# - export type { typed } from './x' (type re-export)
# Check if there's a "type" keyword in the export_statement children
sed -n '283,310p' src/parser/outline.tsRepository: flyingrobots/graft
Length of output: 3722
Type re-exports lack semantic distinction from value re-exports.
The code extracts names from export type { ... } from '...' statements via the export_clause handler, but assigns them kind: "export" identically to value re-exports. Tree-sitter likely produces a single export_statement node type for both forms, with the type keyword as a child node. The handler should check for this keyword—similar to the hasWildcard pattern on line 304—and assign kind: "type" for type-only re-exports to preserve semantic distinction.
The test at lines 128–133 only validates name extraction via toBeDefined() without verifying the kind field, masking this incomplete categorization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/parser/outline.ts` around lines 285 - 310, The export_clause handling
currently treats all re-exports as kind: "export"; update the loop that
processes export_specifier nodes (in the exportClause branch) to detect if the
clause is a type-only re-export by checking for a child whose type/text is
"type" (similar to the hasWildcard pattern) and set entries.push(... { kind:
"type", ... }) and buildJumpEntry(..., "type", ...) when present; leave value
re-exports as kind: "export". Also update the related test assertions (the test
around lines 128–133) to assert the kind field (expecting "type" for type
re-exports) rather than only toBeDefined().
| // Audit fixture: Re-export patterns (barrel file) | ||
| // Tests: named re-exports, type re-exports, wildcard re-exports | ||
|
|
||
| export { UserCard, UserList } from "./react-component"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the export forms in the source module and the barrel.
printf 'react-component exports:\n'
rg -n --no-heading '^\s*export\b' test/fixtures/audit/react-component.tsx
printf '\nre-exports barrel entries:\n'
rg -n --no-heading '^\s*export\b' test/fixtures/audit/re-exports.tsRepository: flyingrobots/graft
Length of output: 1018
Fix invalid default re-export syntax in barrel.
Line 4 misrepresents the module contract. react-component.tsx exports UserCard exclusively as the module default (line 47), not as a named export. The barrel re-exports it as if it were named, which is a lie the AST won't tolerate in production. UserList is correctly a named export (line 66).
Rewrite the re-export to respect the actual module shape:
Fix
-export { UserCard, UserList } from "./react-component";
+export { default as UserCard, UserList } from "./react-component";📝 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
| export { UserCard, UserList } from "./react-component"; | |
| export { default as UserCard, UserList } from "./react-component"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/fixtures/audit/re-exports.ts` at line 4, The barrel re-exports the
default-exported component as a named export: replace the incorrect named
re-export of UserCard with a default-to-named re-export that respects the module
shape from "./react-component" (export the module default as UserCard) while
keeping UserList as the named re-export; update the export statement in
re-exports.ts to re-export the module default as UserCard and also export
UserList by name so the AST reflects the actual exports of react-component.tsx.
Comment on lines
+79
to
+86
| it("extracts both middleware functions as kind=function", () => { | ||
| expect(byName(result, "authenticate")!.kind).toBe("function"); | ||
| expect(byName(result, "rateLimit")!.kind).toBe("function"); | ||
| }); | ||
|
|
||
| it("extracts RouteParams interface and HandlerFn type", () => { | ||
| expect(byName(result, "RouteParams")!.kind).toBe("interface"); | ||
| expect(byName(result, "HandlerFn")!.kind).toBe("type"); |
There was a problem hiding this comment.
Missing existence assertions before property access.
Lines 80-81 and 85-86 access .kind directly on byName() results without first asserting the entry exists. If authenticate or RouteParams is missing from the outline, you get the cryptic:
TypeError: Cannot read properties of undefined (reading 'kind')
Instead of a clear "expected undefined to be defined". Add explicit existence checks like the other tests do.
💚 Proposed fix
it("extracts both middleware functions as kind=function", () => {
- expect(byName(result, "authenticate")!.kind).toBe("function");
- expect(byName(result, "rateLimit")!.kind).toBe("function");
+ const auth = byName(result, "authenticate");
+ const rate = byName(result, "rateLimit");
+ expect(auth).toBeDefined();
+ expect(rate).toBeDefined();
+ expect(auth!.kind).toBe("function");
+ expect(rate!.kind).toBe("function");
});
it("extracts RouteParams interface and HandlerFn type", () => {
- expect(byName(result, "RouteParams")!.kind).toBe("interface");
- expect(byName(result, "HandlerFn")!.kind).toBe("type");
+ const routeParams = byName(result, "RouteParams");
+ const handlerFn = byName(result, "HandlerFn");
+ expect(routeParams).toBeDefined();
+ expect(handlerFn).toBeDefined();
+ expect(routeParams!.kind).toBe("interface");
+ expect(handlerFn!.kind).toBe("type");
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/parser/outline-audit.test.ts` around lines 79 - 86, The tests
"extracts both middleware functions as kind=function" and "extracts RouteParams
interface and HandlerFn type" access byName(result, ...)!.kind without asserting
the entry exists; add explicit existence assertions (e.g., expect(byName(result,
"authenticate")).toBeDefined(), expect(byName(result,
"rateLimit")).toBeDefined(), expect(byName(result,
"RouteParams")).toBeDefined(), expect(byName(result,
"HandlerFn")).toBeDefined()) before reading .kind so failures produce a clear
"expected to be defined" message and avoid TypeError when byName returns
undefined.
Comment on lines
+117
to
+134
| describe("audit: re-exports", () => { | ||
| const result = loadAndParse("re-exports.ts"); | ||
|
|
||
| it("extracts local declarations alongside re-exports", () => { | ||
| expect(byName(result, "createApp")).toBeDefined(); | ||
| expect(byName(result, "createApp")!.kind).toBe("function"); | ||
| }); | ||
|
|
||
| it("extracts named re-exports", () => { | ||
| expect(byName(result, "UserCard")).toBeDefined(); | ||
| expect(byName(result, "authenticate")).toBeDefined(); | ||
| }); | ||
|
|
||
| it("extracts type re-exports", () => { | ||
| expect(byName(result, "UserProps")).toBeDefined(); | ||
| expect(byName(result, "RouteParams")).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Test "extracts named re-exports" doesn't verify the kind is "export".
You assert UserCard and authenticate are defined, and you assert UserProps and RouteParams are defined for type re-exports. But you never verify their kind is "export". If the parser incorrectly classified them as "function" or "interface", these tests would pass anyway.
💚 Proposed fix
it("extracts named re-exports", () => {
- expect(byName(result, "UserCard")).toBeDefined();
- expect(byName(result, "authenticate")).toBeDefined();
+ const userCard = byName(result, "UserCard");
+ const auth = byName(result, "authenticate");
+ expect(userCard).toBeDefined();
+ expect(userCard!.kind).toBe("export");
+ expect(auth).toBeDefined();
+ expect(auth!.kind).toBe("export");
});
it("extracts type re-exports", () => {
- expect(byName(result, "UserProps")).toBeDefined();
- expect(byName(result, "RouteParams")).toBeDefined();
+ const userProps = byName(result, "UserProps");
+ const routeParams = byName(result, "RouteParams");
+ expect(userProps).toBeDefined();
+ expect(userProps!.kind).toBe("export");
+ expect(routeParams).toBeDefined();
+ expect(routeParams!.kind).toBe("export");
});📝 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
| describe("audit: re-exports", () => { | |
| const result = loadAndParse("re-exports.ts"); | |
| it("extracts local declarations alongside re-exports", () => { | |
| expect(byName(result, "createApp")).toBeDefined(); | |
| expect(byName(result, "createApp")!.kind).toBe("function"); | |
| }); | |
| it("extracts named re-exports", () => { | |
| expect(byName(result, "UserCard")).toBeDefined(); | |
| expect(byName(result, "authenticate")).toBeDefined(); | |
| }); | |
| it("extracts type re-exports", () => { | |
| expect(byName(result, "UserProps")).toBeDefined(); | |
| expect(byName(result, "RouteParams")).toBeDefined(); | |
| }); | |
| }); | |
| describe("audit: re-exports", () => { | |
| const result = loadAndParse("re-exports.ts"); | |
| it("extracts local declarations alongside re-exports", () => { | |
| expect(byName(result, "createApp")).toBeDefined(); | |
| expect(byName(result, "createApp")!.kind).toBe("function"); | |
| }); | |
| it("extracts named re-exports", () => { | |
| const userCard = byName(result, "UserCard"); | |
| const auth = byName(result, "authenticate"); | |
| expect(userCard).toBeDefined(); | |
| expect(userCard!.kind).toBe("export"); | |
| expect(auth).toBeDefined(); | |
| expect(auth!.kind).toBe("export"); | |
| }); | |
| it("extracts type re-exports", () => { | |
| const userProps = byName(result, "UserProps"); | |
| const routeParams = byName(result, "RouteParams"); | |
| expect(userProps).toBeDefined(); | |
| expect(userProps!.kind).toBe("export"); | |
| expect(routeParams).toBeDefined(); | |
| expect(routeParams!.kind).toBe("export"); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/parser/outline-audit.test.ts` around lines 117 - 134, The "extracts
named re-exports" test currently only checks presence of UserCard and
authenticate; update that test to assert their kind is "export" by using the
byName(result, "<symbol>") lookup (e.g., for UserCard and authenticate) and
adding assertions that their .kind equals "export" so re-exports can't be
misclassified as "function" or other kinds; modify the same test case that
references result and byName to include these kind checks.
This was referenced Apr 4, 2026
3 tasks
This was referenced May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Test plan
pnpm test— 267 tests pass (21 files)pnpm lint— cleanpnpm typecheck— cleanCloses CORE_outline-quality-audit ASAP ticket.