[feature] AST-Based Logical Density Metrics (Cyclomatic Complexity, Block Nesting Depth, LOC Refinement)#30
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds AST-derived complexity and LOC metrics to parser entities, persists the new fields in SQLite, and updates graph lookup code to read ChangesCode Metrics Pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db/db.cpp`:
- Around line 12-23: The schema change in db.cpp updates the file table
definition to use raw_loc, but existing databases created by initDb() will still
have the old loc column because CREATE TABLE IF NOT EXISTS does not migrate
tables. Add a migration path alongside the file table setup in db.cpp that
detects the old schema and either ALTER TABLE the existing file table or
recreates/backfills it so loc data is preserved as raw_loc. Make sure the
migration is wired into the same initialization flow that populates the file
table used by the inserter and graph queries, so older databases continue to
work without missing LOC values.
In `@src/parser/ParseResult.h`:
- Around line 10-12: Default-initialize the new ParseResult fields raw_loc,
logical_loc, and is_generated in ParseResult.h so a default-constructed
ParseResult is always safe. Update the ParseResult struct/class definition
alongside the existing rollup fields so PythonParser::parseFile’s early return
path cannot leave these members indeterminate before DbInserter::insertFile
binds them. Keep the change scoped to the ParseResult member declarations and
preserve the existing zero-initialized behavior used by the other loc counters.
🪄 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: c9570037-3fc6-49a2-98ca-ef06b558d8bd
📒 Files selected for processing (5)
src/algo/Graph.cppsrc/db/DbInserter.cppsrc/db/db.cppsrc/parser/ParseResult.hsrc/parser/PythonParser.cpp
| " file_id TEXT PRIMARY KEY," | ||
| " file_name TEXT NOT NULL," | ||
| " language TEXT NOT NULL," | ||
| " raw_loc INTEGER NOT NULL," | ||
| " logical_loc INTEGER NOT NULL DEFAULT 0," | ||
| " is_generated INTEGER NOT NULL DEFAULT 0," | ||
| " max_cyclomatic_complexity INTEGER NOT NULL DEFAULT 0," | ||
| " avg_cyclomatic_complexity REAL NOT NULL DEFAULT 0.0," | ||
| " max_block_depth INTEGER NOT NULL DEFAULT 0," | ||
| " avg_block_depth REAL NOT NULL DEFAULT 0.0," | ||
| " max_function_loc INTEGER NOT NULL DEFAULT 0," | ||
| " avg_function_loc REAL NOT NULL DEFAULT 0.0" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add a migration path for existing databases.
CREATE TABLE IF NOT EXISTS will leave existing file tables unchanged, so databases created with the old loc column will still lack raw_loc. The updated inserter and graph query then fail or silently skip file LOC data. Add an ALTER TABLE/schema-version migration or recreate/backfill strategy for loc → raw_loc.
Possible migration sketch
+ // After creating/opening schema, migrate older DBs that still have file.loc.
+ // Prefer a schema_version/user_version based migration if this project already has one.
+ // Example only:
+ // ALTER TABLE file RENAME COLUMN loc TO raw_loc;
+ // ALTER TABLE file ADD COLUMN logical_loc INTEGER NOT NULL DEFAULT 0;
+ // ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/db/db.cpp` around lines 12 - 23, The schema change in db.cpp updates the
file table definition to use raw_loc, but existing databases created by initDb()
will still have the old loc column because CREATE TABLE IF NOT EXISTS does not
migrate tables. Add a migration path alongside the file table setup in db.cpp
that detects the old schema and either ALTER TABLE the existing file table or
recreates/backfills it so loc data is preserved as raw_loc. Make sure the
migration is wired into the same initialization flow that populates the file
table used by the inserter and graph queries, so older databases continue to
work without missing LOC values.
| int raw_loc; // total line count (including blanks and comments) | ||
| int logical_loc; // non-blank, non-comment line count | ||
| int is_generated; // 1 if vendored/generated, 0 otherwise |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Default-initialize raw_loc, logical_loc, and is_generated.
These three new int members are left uninitialized, unlike the six rollup fields below that you defaulted to 0. PythonParser::parseFile returns a default-constructed ParseResult on the file-open-failure early-return (return result; at line 753) before any of these are assigned. That result then flows to DbInserter::insertFile, which binds the garbage values into raw_loc/logical_loc/is_generated (all NOT NULL columns), persisting indeterminate data.
🛡️ Proposed fix
- int raw_loc; // total line count (including blanks and comments)
- int logical_loc; // non-blank, non-comment line count
- int is_generated; // 1 if vendored/generated, 0 otherwise
+ int raw_loc = 0; // total line count (including blanks and comments)
+ int logical_loc = 0; // non-blank, non-comment line count
+ int is_generated = 0; // 1 if vendored/generated, 0 otherwise📝 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.
| int raw_loc; // total line count (including blanks and comments) | |
| int logical_loc; // non-blank, non-comment line count | |
| int is_generated; // 1 if vendored/generated, 0 otherwise | |
| int raw_loc = 0; // total line count (including blanks and comments) | |
| int logical_loc = 0; // non-blank, non-comment line count | |
| int is_generated = 0; // 1 if vendored/generated, 0 otherwise |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/parser/ParseResult.h` around lines 10 - 12, Default-initialize the new
ParseResult fields raw_loc, logical_loc, and is_generated in ParseResult.h so a
default-constructed ParseResult is always safe. Update the ParseResult
struct/class definition alongside the existing rollup fields so
PythonParser::parseFile’s early return path cannot leave these members
indeterminate before DbInserter::insertFile binds them. Keep the change scoped
to the ParseResult member declarations and preserve the existing
zero-initialized behavior used by the other loc counters.
Summary
Closes #29
Implements per-function and per-file logical density metrics across three dimensions: cyclomatic complexity, block nesting depth, and line-of-code counts. Adds generated/vendored file detection and a structural safety net for zero-function files. Also fixes a build-breaking comment in
src/parser/ParseResult.hwhere a// \line continuation silently removedavg_cyclomatic_complexityfrom the struct.Commits
feat(parser): compute cyclomatic complexity per functionPythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfeat(db): roll up cyclomatic complexity to file level (max/avg)PythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfeat(parser): compute block nesting depth per functionPythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfeat(parser): handle comprehension clause depth as sequential nestingPythonParser.cppfeat(db): roll up block nesting depth to file level (max/avg)PythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfeat(parser): add per-function and per-file LOCPythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfeat(parser): add logical LOC alongside existing raw LOCPythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cpp,Graph.cppfeat(parser): handle generated/vendored file exclusion or flaggingPythonParser.cpp,ParseResult.h,db.cpp,DbInserter.cppfix(parser): use segment split for generated/vendored path detectionPythonParser.cpprefactor(parser): zero-initialize FileEntity rollup fields as structural safety netParseResult.hfix(parser): remove line-continuation backslash from FileEntity commentParseResult.hFiles Changed
src/parser/ParseResult.hFileEntity: renamedloc→raw_loc; addedlogical_loc,is_generated, and six function-derived rollup fields (max/avg_cyclomatic_complexity,max/avg_block_depth,max/avg_function_loc) with default member initializersFunctionEntity: addedcyclomatic_complexity,max_block_depth,locsrc/parser/PythonParser.cppcountCyclomaticComplexity(TSNode)— new; counts branch-type nodes recursively, stops at function/lambda boundariescomputeMaxBlockDepthClauses(TSNode, int, int&)— new; handles comprehension clause depth sequentiallycomputeMaxBlockDepth(TSNode, int, int&)— new; increments on control-flow constructs, dispatches to clauses helper for comprehensionshandleFunctionDef()— computescc,maxBlockDepth,locper functionparseFile()— adds logical LOC pass, generated/vendored detection, and file-level rollup blocksrc/db/db.cppfunctiontable: addedcyclomatic_complexity,max_block_depth,loccolumnsfiletable: renamedloc→raw_loc; addedlogical_loc,is_generated, and six rollup columnssrc/db/DbInserter.cppINSERTstatements andsqlite3_bind_*calls updated for bothfunction(8 → 11 params) andfile(3 → 12 params)src/algo/Graph.cppbuild_extra_maps(): SQL updated fromSELECT file_id, loc FROM file→SELECT file_id, raw_loc FROM file(required by theloc→raw_locrename in commit 7)Design Decisions
Cyclomatic Complexity
Lambdas are function boundaries.
countCyclomaticComplexitystops recursion at bothfunction_definitionandlambdanodes. A conditional insidef = lambda x: x if x > 0 else -xdoes not contribute to the outer function's CC. Mirrors the pattern used incollectCalls. To change: remove thelambdastop condition incountCyclomaticComplexity.Comprehension
for/ifclauses are not counted.for_in_clauseandif_clauseinside list/set/dict comprehensions and generators are not in the branch-type list. The spec covers comprehensions under block nesting depth, not under CC. To change: addfor_in_clauseandif_clauseto the branch-type list incountCyclomaticComplexity.elseis not counted.else_clauseonif,for,while, andtrydoes not add +1. Onlyif_statement,elif_clause,for_statement,while_statement,case_clause,except_clause,conditional_expression, andboolean_operatorcontribute. Standard McCabe:elseis the implicit fallthrough, not a decision point.Block Nesting Depth
elif/else/except/finallyinherit parent depth.These are children of their parent construct (
if_statement,try_statement,match_statement) and are recursed into at the parent's already-incremented depth. They do not increment depth again.Comprehension clauses use sequential accumulation (
computeMaxBlockDepthClauses).Tree-sitter Python represents
for_in_clauseandif_clauseas siblings inside a comprehension node — not nested.computeMaxBlockDepthClausesthreads depth through them sequentially so[x for x in items if x.valid]correctly reaches depth 2, equivalent to aforcontaining anif, rather than depth 1.Nested functions and lambdas reset the counter.
Both
computeMaxBlockDepthandcomputeMaxBlockDepthClausesstop atfunction_definitionandlambdanodes. The inner function gets its own counter starting from 0.LOC
Function LOC is
end_line − start_line + 1(raw line count).Includes blank lines and comments within the function body. Lines are 0-indexed from tree-sitter, so a function spanning rows 0–1 has
loc = 2.raw_locvslogical_loc.raw_locis total line count.logical_locskips blank lines and lines where the first non-whitespace character is#.raw_locis preserved for the graph scoring pipeline (Graph.cpp);logical_locis the intended input for complexity density calculations.Docstrings are not excluded from
logical_loc.The filter only strips
#-prefixed lines and blanks."""..."""multi-line strings count as logical lines. A grep across all.pyfiles in this repo found no generation markers in docstrings, so this gap does not affect any current file.Generated/Vendored Detection
Flagging, not exclusion.
Files are parsed and stored normally with
is_generated = 1. The downstream percentile-ranking step (separate issue) filters onWHERE is_generated = 0. Exclusion was rejected because it makes files invisible to the UI and is harder to reverse.Path-segment matching, not substring.
file_idis split by/and each directory segment (filename excluded) is matched in full against:vendor,_vendor,third_party,node_modules,venv,.venv,site-packages,__pycache__,dist,build,out. Full-segment matching avoids false positives from filenames likevendor_utils.py.buildandoutcarry moderate false-positive risk for projects that use those as source directories.Content signal:
#-prefixed first 5 lines, case-insensitive.If path detection does not fire, the first 5 lines are checked. Lines starting with
#are matched case-insensitively against"generated","do not edit", and"autogenerated". Docstring-style markers are not detected.Zero-Function Files
Structural hardening only — not a live bug fix.
The rollup code already produced correct zeros before this commit (
maxinitialized to 0 before the loop;avgguards withn > 0 ? ... : 0.0). Commit 9 adds default member initializers to the six function-derived fields inFileEntityas a safety net against future code changes.raw_loc,logical_loc, andis_generatedwere intentionally left without defaults — they are unconditionally assigned on every reachable path inparseFile().DB Schema
functiontable (11 columns)filetable (12 columns)How to Test
1. Create test files
Create
C:\tmp\cc_test\test_cc.pywith the following content:C:\tmp\cc_test\generated_file.py— single comment line with a generation marker:C:\tmp\cc_test\imports_only.py— imports only, no functions:2. Build and scan
Expected:
3. Run verification queries
4. Expected output