feat(complexity): cyclomatic complexity column + high-complexity-untested recipe (research note § 1.4)#70
Conversation
…sted recipe
Research note § 1.4 ship-pick (c) per § 5 cadence. Schema bump
SCHEMA_VERSION 7 → 8.
Schema:
- symbols.complexity REAL column. NULL for non-function kinds and
class methods (v1 limitation documented in recipe .md).
Parser:
- complexityStack maintained alongside scopeStack. Function entry
pushes {symbolIndex, count: 1}; branching-node visitors increment
top.count; function exit pops + writes count into the already-
pushed symbol row's complexity field.
- McCabe decision points counted: if, while, do-while, for, for-in,
for-of, case X (not default:), &&/||/??, ?:, catch.
Bundled recipe high-complexity-untested:
- Joins symbols (complexity >= 10) with coverage (< 50%).
- Combines structural + runtime evidence axes — surfaces refactor-
priority candidates that untested-and-dead and worst-covered-exports
miss (they catch dead-or-uncalled, this catches called-but-undertested-
AND-branchy).
Empirical sanity check on codemap's own index after reindex:
- extractFileData (parser.ts main visitor) → complexity 108 ✓
- stringifyTypeNode → 42 ✓
- All non-function kinds have NULL complexity ✓
- high-complexity-untested recipe returns 7 functions all from
src/parser.ts (which has 0% coverage; complexity ≥ 10) ✓
Lockstep updates per Rule 10 (templates/agents + .agents):
- Trigger pattern row "What's high-complexity AND undertested?"
- Quick reference row for SELECT name, complexity FROM symbols
- Recipe-id list extended in SKILL.md
Plus architecture.md (schema version 8, complexity column docs),
glossary.md (cyclomatic complexity entry), patch changeset.
Files changed:
- src/db.ts (SCHEMA_VERSION + symbols.complexity column + insertSymbols
bind + SymbolRow optional complexity field)
- src/parser.ts (complexityStack + branching node visitors + push/pop
in FunctionDeclaration / VariableDeclaration arrow-fn paths)
- templates/recipes/high-complexity-untested.{sql,md}
- docs/architecture.md (schema version + symbols column doc)
- docs/glossary.md (new entry)
- templates/agents/rules/codemap.md + .agents/rules/codemap.md
(trigger + quick-ref rows)
- templates/agents/skills/codemap/SKILL.md + .agents/skills/codemap/
SKILL.md (recipe-id list)
- .changeset/cyclomatic-complexity.md (patch)
Verification:
- bun test: 754 pass
- bun run check passes (format, lint, typecheck, 23/23 golden queries)
- Live re-index against codemap source produces sensible complexity
values (parser visitor itself is the highest at 108, which tracks)
🦋 Changeset detectedLatest commit: 42146cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces McCabe cyclomatic complexity computation for function-shaped symbols in the codemap system. A ChangesCyclomatic Complexity Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
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. Review rate limit: 0/1 reviews remaining, refill in 28 minutes and 40 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/architecture.md (1)
201-216:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the malformed
symbolsschema table row formatting/content.The markdown table structure is broken here (extra
|columns and corrupted code spans), so the schema docs render incorrectly and the complexity description becomes unreliable.🛠️ Suggested patch
-| ----------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --- | ----------------------------------------------------------------------------------------------------- | +| ----------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | @@ -| complexity | REAL | Cyclomatic complexity (McCabe; `1 + decision-points`) for function-shaped symbols only. NULL for non-functions (interfaces, types, enums, plain consts) and class methods (v1 limitation). Decision points: `if`/`while`/`do…while`/`for`/`for…in`/`for…of`, `case X:` arms (not `default:`), `&&`/` | | `/`??`short-circuit operators,`?:`ternary,`catch`clauses. Powers the`high-complexity-untested` recipe | +| complexity | REAL | Cyclomatic complexity (McCabe; `1 + decision points`) for function-shaped symbols only. NULL for non-functions (interfaces, types, enums, plain consts) and class methods (v1 limitation). Decision points: `if`, `while`, `do…while`, `for`, `for…in`, `for…of`, `case X:` arms (not `default:`), `&&`, `||`, `??`, ternary `?:`, and `catch`. Powers the `high-complexity-untested` recipe. |As per coding guidelines
**/*.md: Documentation should be written in clear, concise English.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture.md` around lines 201 - 216, The "symbols" schema table row is malformed around the complexity column—extra pipe characters and corrupted code spans broke the markdown table and truncated the complexity description; fix the table by restoring a single well-formed row for the complexity column (matching the other rows' 5-column layout), remove the stray/backtick artifacts, and reflow the full complexity description text (include decision points: if/while/do…while/for/for…in/for…of, case arms, &&/|| short-circuit, ?: ternary, catch clauses) so the sentence after the `complexity` header reads cleanly; update the impacted doc block near the `complexity`, `visibility`, and `signature` entries to ensure the pipe counts and inline code spans are correct and the table renders properly.src/parser.ts (1)
203-267:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftComplexity misattribution for multi-declaration variable statements.
When a single
VariableDeclarationcontains multiple arrow/function declarations (e.g.,const a = () => { if(x){} }, b = () => {};), all symbols are pushed tocomplexityStackin the entry handler's loop before the visitor traverses any function bodies. This causes branches in earlier functions to increment the complexity counter of later functions.Example:
const a = () => { if(x){} }, b = () => { if(y){} };
- Entry handler pushes both
{symbolIndex: 0, count: 1}and{symbolIndex: 1, count: 1}- Visitor traverses
a's body →IfStatementincrements top →b's count becomes 2- Visitor traverses
b's body →IfStatementincrements top →b's count becomes 3- Exit pops:
symbols[1].complexity = 3(wrong),symbols[0].complexity = 1(wrong)Consider adding
ArrowFunctionExpression/FunctionExpressionhandlers to push/pop complexity per-function rather than per-declaration-statement, or process declarations one-at-a-time with immediate child visitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/parser.ts` around lines 203 - 267, The VariableDeclaration handler currently pushes complexity entries for all declarations up front (see VariableDeclaration, scopePush, pushComplexityFor) which misattributes branch increments when multiple arrow/function declarations exist; fix by pushing and popping complexity in the function-level visitors instead: remove or avoid pushComplexityFor from the bulk VariableDeclaration loop and instead implement/extend ArrowFunctionExpression and FunctionExpression enter/exit handlers to call scopePush, pushComplexityFor (on enter) and scopePop, popComplexityInto (on exit), and ensure maybeAddComponent/currentFunctionScope handling still runs for component detection; alternatively process each declaration one-at-a-time by traversing its init/body immediately inside VariableDeclaration so complexityStack entries are scoped per-function rather than per-declaration-statement.
🧹 Nitpick comments (1)
src/parser.ts (1)
144-150: 💤 Low valueGuard in
popComplexityIntois a no-op as currently called.The function accepts
symbolIndexand guardstop.symbolIndex === symbolIndex, but all call sites passtop.symbolIndexitself (lines 196, 260), making the check always true. Either:
- Remove the parameter and always pop the top, or
- Store the original
symbolIndexfrom entry to pass at exit for a real guardThis doesn't affect correctness today but is confusing and doesn't provide the defensive check it appears to.
♻️ Option 1: Simplify to always pop top
- const popComplexityInto = (symbolIndex: number) => { - const top = complexityStack[complexityStack.length - 1]; - if (top && top.symbolIndex === symbolIndex) { - symbols[symbolIndex].complexity = top.count; - complexityStack.pop(); - } - }; + const popComplexity = () => { + const top = complexityStack.pop(); + if (top) { + symbols[top.symbolIndex].complexity = top.count; + } + };Then update call sites to just call
popComplexity().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/parser.ts` around lines 144 - 150, The guard in popComplexityInto is redundant because callers pass top.symbolIndex; simplify by replacing popComplexityInto(symbolIndex) with a parameterless popComplexity() that always reads the top of complexityStack, assigns symbols[top.symbolIndex].complexity = top.count (if top exists), then complexityStack.pop(); update all call sites (currently calling popComplexityInto with top.symbolIndex) to call popComplexity() and ensure the function still checks for existence of top before accessing its fields; keep references to complexityStack and symbols[top.symbolIndex] unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/codemap/SKILL.md:
- Line 37: The documentation is missing the symbols.complexity column used by
the high-complexity-untested recipe; update the schema section that defines the
symbols table to include a brief entry for complexity (e.g., data type and
meaning) so agents know the field exists and how it’s used by the
high-complexity-untested recipe; locate the symbols table definition and add the
symbols.complexity description alongside other symbol columns.
---
Outside diff comments:
In `@docs/architecture.md`:
- Around line 201-216: The "symbols" schema table row is malformed around the
complexity column—extra pipe characters and corrupted code spans broke the
markdown table and truncated the complexity description; fix the table by
restoring a single well-formed row for the complexity column (matching the other
rows' 5-column layout), remove the stray/backtick artifacts, and reflow the full
complexity description text (include decision points:
if/while/do…while/for/for…in/for…of, case arms, &&/|| short-circuit, ?: ternary,
catch clauses) so the sentence after the `complexity` header reads cleanly;
update the impacted doc block near the `complexity`, `visibility`, and
`signature` entries to ensure the pipe counts and inline code spans are correct
and the table renders properly.
In `@src/parser.ts`:
- Around line 203-267: The VariableDeclaration handler currently pushes
complexity entries for all declarations up front (see VariableDeclaration,
scopePush, pushComplexityFor) which misattributes branch increments when
multiple arrow/function declarations exist; fix by pushing and popping
complexity in the function-level visitors instead: remove or avoid
pushComplexityFor from the bulk VariableDeclaration loop and instead
implement/extend ArrowFunctionExpression and FunctionExpression enter/exit
handlers to call scopePush, pushComplexityFor (on enter) and scopePop,
popComplexityInto (on exit), and ensure maybeAddComponent/currentFunctionScope
handling still runs for component detection; alternatively process each
declaration one-at-a-time by traversing its init/body immediately inside
VariableDeclaration so complexityStack entries are scoped per-function rather
than per-declaration-statement.
---
Nitpick comments:
In `@src/parser.ts`:
- Around line 144-150: The guard in popComplexityInto is redundant because
callers pass top.symbolIndex; simplify by replacing
popComplexityInto(symbolIndex) with a parameterless popComplexity() that always
reads the top of complexityStack, assigns symbols[top.symbolIndex].complexity =
top.count (if top exists), then complexityStack.pop(); update all call sites
(currently calling popComplexityInto with top.symbolIndex) to call
popComplexity() and ensure the function still checks for existence of top before
accessing its fields; keep references to complexityStack and
symbols[top.symbolIndex] 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4f3314d-1a4e-4dc3-b325-e0743710d580
📒 Files selected for processing (11)
.agents/rules/codemap.md.agents/skills/codemap/SKILL.md.changeset/cyclomatic-complexity.mddocs/architecture.mddocs/glossary.mdsrc/db.tssrc/parser.tstemplates/agents/rules/codemap.mdtemplates/agents/skills/codemap/SKILL.mdtemplates/recipes/high-complexity-untested.mdtemplates/recipes/high-complexity-untested.sql
CodeRabbit catch on PR #70: the high-complexity-untested recipe row was added to .agents/skills/codemap/SKILL.md but the symbols table schema section (under "### `symbols` — Functions, types, ...") still listed columns through `visibility` only, missing the new `complexity REAL` column. Verified by reading the file — claim was correct. Both lockstep mirrors (.agents/ + templates/agents/) updated with the same row: | complexity | REAL | Cyclomatic complexity (`1 + decision points`) for function-shaped symbols. NULL for non-functions and class methods (v1). Powers --recipe high-complexity-untested. Decision points: if, while, do…while, for/for-in/for-of, case X: (not default:), &&/||/??/?:, catch | Per docs/README.md Rule 10 — agent rule + skill schema docs must stay in lockstep with code-side schema changes. The trigger-pattern row + recipe-id list were already updated; the schema-table row was the gap.
…ution + cleanups CodeRabbit raised three valid findings on PR #70. All fact-checked against the code; all correct. A) docs/architecture.md symbols schema table was malformed: - Markdown table separator row had extra `| --- | ---` segments because oxfmt mis-counted columns when the description contained `|` chars inside `&&`/`||`/`??` backtick spans. - The complexity row's description was split across THREE cells with broken backtick fences. - Fix: restored single-row layout (3 cells: Column | Type | Description) and rephrased the decision-point list to avoid `|` inside backticks ("short-circuit `&&` / `||` / `??`" instead of "`&&`/`||`/`??`"). B) src/parser.ts complexity misattribution on multi-declarator VariableDeclaration (e.g. `const a = () => {…}, b = () => {…};`): Pre-fix: VariableDeclaration enter pushed all declarators' complexity entries up front. Then visitor traversed `a`'s body — branches incremented top (= b's entry). Then `b`'s body. Exit pops in reverse → symbols[1].complexity = 3 (wrong), symbols[0].complexity = 1 (wrong). Real bug. Fix: push/pop complexity on the FUNCTION-shaped node visitors (ArrowFunctionExpression / FunctionExpression) — not on VariableDeclaration. The VariableDeclaration handler still creates the symbol row but only RECORDS the symbol → init-node mapping in a WeakMap. The ArrowFunctionExpression / FunctionExpression enter handler reads the WeakMap to know which symbol to write back to; anonymous arrow fns (callbacks, IIFEs) get -1 and just track count without persistence. Verified against fixture: const a = () => { if (1===1) {…} }, b = () => { if (2===2) {…} }, c = () => 5; → a=2, b=2, c=1 (correct; pre-fix was a=1, b=3, c=1) C) popComplexityInto guard was a no-op (callers passed top.symbolIndex, so the equality check was always true). Simplified to parameterless popComplexityTop() that always pops + writes back if symbolIndex >= 0. Folds naturally into the B refactor — every push/pop pair now lives in a function-shaped visitor. Also re-ran codemap query against codemap source post-fix: extractFileData=108, stringifyTypeNode=42, extractClassMembers=18, extractLiteralValue=15, extractObjectMembers=14 Same scores as pre-fix on these (no FunctionExpression / arrow nesting in those particular functions, so the bug didn't surface) — confirms the refactor is a strict improvement, not a regression.
|
All three review findings fact-checked against the code; all correct, all fixed in 320b827.
Bug fix verification (B) — pre-fix vs post-fix on the exact case CodeRabbit flagged: const a = () => { if (1===1) {…} },
b = () => { if (2===2) {…} },
c = () => 5;
No regression check — re-ran complexity recipe against codemap source post-fix: |
… on parser.ts Fact-checking against codebase post-PR-#69-and-#70 surfaced four stale spots; concise-comments rule re-applied to recently-authored parser.ts comments. DOCS LIFTED (post-FTS5 / Mermaid / complexity merge): - README.md (root) line 113 — --format enum was missing `mermaid`. Updated to <text|json|sarif|annotations|mermaid> + added the bounded-input contract one-liner + 50-edge ceiling note. Added --with-fts example block alongside (was missing entirely; README is the canonical CLI surface per docs/README.md Single source of truth table). - docs/architecture.md output-formatters paragraph — described only formatSarif + formatAnnotations; missing formatMermaid + bounded- input contract. Added formatMermaid description + MERMAID_MAX_EDGES reference + the no-auto-truncation reasoning (would be a verdict masquerading as output mode). Updated the --format CLI enum to include mermaid; same for the MCP tools format union. - .agents/skills/codemap/SKILL.md + templates/agents/skills/codemap/ SKILL.md — recipe-id list missed three coverage recipes (untested-and-dead, files-by-coverage, worst-covered-exports) shipped earlier in PR #65/#56 era. Lockstep update per Rule 10. Skill now lists 20 of 20 bundled recipe ids. CONCISE-COMMENTS SWEEP on parser.ts (recently authored): - Trimmed the 14-line complexityStack JSDoc block to 6 lines. Kept: the -1 sentinel rationale (non-obvious), the WeakMap rationale (the bug fix from PR #70 review). Cut: re-stating push/pop semantics obvious from method names + step-by-step "this then that" prose. - Removed the "Defer complexity push to..." comment in the VariableDeclaration handler. The 4-line block restated the design decision documented one screen up in the complexityStack jsdoc; cross-ref makes it redundant. Per concise-comments § "Cut" rule: "Cross-references that save grep time" — keep when they actually do; cut when they restate. Verification: - bun run check: format + lint + typecheck + 23/23 golden ✓ - Recipe count: SQL files = 20, skill mentions = 20 (1:1 match) ✓ - SCHEMA_VERSION = 8 in db.ts; docs/architecture.md says 8 ✓ - complexity column documented in architecture.md + glossary.md ✓ - --with-fts in README.md + architecture.md + glossary.md + roadmap.md (consumer-facing surfaces all aligned) ✓ - --format mermaid in README.md + architecture.md + glossary.md + agent rule/skill ✓
Summary
Research note § 1.4 ship-pick (c) per the § 5 cadence — next after (a) FTS5+Mermaid landed in PR #69.
Ships per-function cyclomatic complexity computed during AST walking. Schema bump
SCHEMA_VERSION7 → 8.What lands
symbols.complexity REALcolumn. Computed by parser walker per McCabe formula (1 + decision points).if,while,do…while,for,for…in,for…of,case X:arms (notdefault:fall-through),&&/||/??short-circuit operators,?:ternary,catchclauses.functiondeclarations + arrow-functionconsts.NULLfor non-functions (interfaces, types, enums, plain consts) and class methods (v1 limitation; documented in recipe.md).high-complexity-untested— joinssymbols.complexity ≥ 10withcoverage.coverage_pct < 50for refactor-priority signal that single-axis recipes miss.Implementation
Parser visitor (
src/parser.ts) maintains acomplexityStackkeyed by symbol index:{symbolIndex, count: 1}(the function-entry path).incrementComplexity(top of stack only — nested functions don't bleed into outer counts).symbols[symbolIndex].complexity = top.count).Empirical sanity check (against codemap's own source)
Highest-complexity function is the parser's own
extractFileDatavisitor — tracks; it's a 500-lineVisitorconstructor with hundreds of branches.Doc-governance compliance
templates/agents/AND.agents/codemap rule + skill updated:SELECT name, complexity FROM symbols WHERE name = '...'SKILL.mddocs/architecture.md— schema version 7→8,complexitycolumn documented insymbolstable.docs/glossary.md— new "cyclomatic complexity / McCabe" entry.SCHEMA_VERSIONmismatch).Test plan
bun test— 754 passbun run check— format, lint, typecheck, 23/23 golden queriessrc/parser.ts(parser visitor itself, ironically)Out of scope (intentional follow-ups)
MethodDefinitionvisitor doesn't push to the complexity stack yet. Documented in the recipe.mdv1 limitation; refactor opportunity for class-heavy projects.complexityis per-symbol; project-local recipes canSUM/AVG/MAXit as needed.docs/plans/c9-plugin-layer.md.Summary by CodeRabbit
New Features
Documentation