Skip to content

feat: build metadata tracking and excludeTests config shorthand#105

Merged
carlos-alm merged 1 commit into
mainfrom
feat/build-metadata-excludetests
Feb 26, 2026
Merged

feat: build metadata tracking and excludeTests config shorthand#105
carlos-alm merged 1 commit into
mainfrom
feat/build-metadata-excludetests

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Build metadata tracking (9.3): New build_meta table (migration v7) stores engine name, version, codegraph version, and build timestamp. Incremental builds now warn when the engine or codegraph version has changed since the last build, suggesting --no-incremental.
  • Build metadata in info command (9.4): codegraph info displays build metadata from the DB and warns on version/engine mismatches.
  • Top-level excludeTests config shorthand (9.2): { "excludeTests": true } at the top level of .codegraphrc.json is now hoisted into query.excludeTests. Nested form takes precedence.

Test plan

  • 3 new unit tests for excludeTests hoisting (top-level true, nested precedence, top-level false)
  • 4 new unit tests for build_meta helpers (table creation, null on missing table, read/write, upsert)
  • All 249 unit tests pass
  • Lint clean

Track engine name, version, and codegraph version in a new build_meta
table (migration v7). Warn during incremental builds when the engine or
codegraph version has changed since the last build, suggesting
--no-incremental. Display build metadata in the info command with
mismatch warnings.

Accept excludeTests at the top level of .codegraphrc.json as a
shorthand for query.excludeTests, with the nested form taking
precedence.

Impact: 4 functions changed, 8 affected
@claude
Copy link
Copy Markdown

claude Bot commented Feb 25, 2026

Claude finished @carlos-alm's task —— View job


Root Cause Analysis

Problem Being Solved: This PR addresses two legitimate UX issues:

  1. Inconsistent builds after engine/version changes - When users switch between native/WASM engines or upgrade codegraph, incremental builds could mix metadata from different parsers/versions, leading to inconsistent graph state
  2. User convenience - Top-level excludeTests config requires nesting under query.excludeTests, making simple configs more verbose

Why This Approach: Both solutions are well-justified:

  • Build metadata tracking provides early warnings when engine/version mismatches are detected during incremental builds, suggesting --no-incremental for consistency
  • Config hoisting maintains backward compatibility while improving ergonomics

Risk Assessment: Low risk - both features are additive with proper fallbacks and comprehensive test coverage.

Configuration Changes

No concerning configuration relaxations detected

  • No linting rules disabled
  • No test timeouts increased
  • No coverage thresholds lowered
  • No strict mode flags disabled

Test Integrity

Strong test coverage with no red flags

  • 7 new unit tests added (3 for config hoisting, 4 for build metadata)
  • Tests cover edge cases: precedence rules, missing tables, upsert behavior
  • No assertions weakened or removed
  • All 249 existing tests still pass
  • Tests follow existing patterns and quality standards

Code Quality Review

Build Metadata Implementation (src/builder.js:355-368, src/db.js:115-132)

Strengths:

  • Clean migration v7 adds build_meta table with proper key-value design
  • Graceful handling of missing table (pre-v7 DBs) via try-catch in getBuildMeta
  • Uses proper transaction for atomic metadata updates
  • Warning messages are helpful and actionable
  • Engine version tracking includes both name and version for completeness

Minor Concerns:

  • Line 958: Debug message on setBuildMeta failure could be upgraded to warn since metadata persistence failures might indicate disk issues

Config Hoisting Implementation (src/config.js:49-52)

Strengths:

  • Correctly implements precedence: nested query.excludeTests beats top-level
  • Proper cleanup removes hoisted property to avoid duplication
  • Boolean coercion ensures type safety

CLI Info Command Enhancement (src/cli.js:672-703)

Strengths:

  • Version mismatch warnings are clear and actionable
  • Proper error handling with try-catch prevents crashes
  • Uses existing getBuildMeta helper consistently

API Exports (src/index.js:27-33)

Strengths:

  • Properly exports new getBuildMeta/setBuildMeta functions for programmatic use
  • Maintains clean separation of concerns

Minor Issues Identified:

  1. src/builder.js:358: The engine comparison prevEngine !== engineName is string-based, but should be robust since getActiveEngine() returns consistent string values

  2. src/builder.js:951: Build metadata persistence happens after graph building completes, but before DB close - good placement for error recovery

  3. Test Coverage: All new functionality has corresponding unit tests with good edge case coverage

Security Considerations

✅ No security concerns identified:

  • No command injection vectors
  • No SQL injection (uses prepared statements)
  • No credential handling changes
  • String comparisons are safe

Performance Impact

✅ Minimal performance overhead:

  • Build metadata operations are single-row key-value operations
  • Config hoisting happens once at startup
  • No impact on hot paths (parsing, querying)

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Well-implemented features solving real user problems with comprehensive test coverage. The build metadata tracking provides valuable consistency warnings, and config hoisting improves UX while maintaining backward compatibility. Code quality is high with proper error handling and clean abstractions. Only minor concern is the debug-level error message for metadata persistence failures.

This PR demonstrates thoughtful engineering with attention to edge cases, proper testing, and user experience improvements.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 25, 2026

Greptile Summary

This PR adds build metadata tracking and a config shorthand for excludeTests. The implementation is clean and well-tested.

Build Metadata Tracking:

  • New build_meta table (migration v7) stores engine, version, and timestamp as key-value pairs
  • Incremental builds warn when engine or codegraph version changes, suggesting --no-incremental for consistency
  • codegraph info displays build metadata and warns on mismatches
  • Backward compatible: gracefully handles pre-v7 databases by returning null

Config Shorthand:

  • { "excludeTests": true } at top level now hoists to query.excludeTests
  • Nested query.excludeTests takes precedence when both are present
  • Top-level property is cleaned up after processing

Testing:

  • 7 new unit tests (3 for config hoisting, 4 for build metadata)
  • All 249 tests pass
  • Tests cover edge cases: missing table, upsert behavior, precedence rules

Code Quality:

  • Proper error handling: metadata failures log debug messages without breaking builds
  • Follows existing patterns: uses transactions for multi-row inserts, try-catch for non-critical operations
  • Clean migration: idempotent with IF NOT EXISTS
  • Clear documentation in README

Confidence Score: 5/5

  • This PR is safe to merge with no concerns
  • Score reflects comprehensive testing (7 new tests, all 249 pass), clean implementation following existing patterns, proper error handling, backward compatibility, and non-breaking changes
  • No files require special attention

Important Files Changed

Filename Overview
src/db.js Adds migration v7 for build_meta table and helper functions getBuildMeta/setBuildMeta with proper error handling
src/builder.js Implements version tracking: reads codegraph version from package.json, checks for engine/version mismatches on incremental builds, persists build metadata after successful builds
src/config.js Implements excludeTests hoisting from top-level to query.excludeTests with correct precedence handling and cleanup
src/cli.js Enhances info command to display build metadata from DB and warn on version/engine mismatches with proper error handling

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[buildGraph starts] --> LoadConfig[Load config with excludeTests hoisting]
    LoadConfig --> CheckIncremental{Incremental build?}
    CheckIncremental -->|Yes| GetMeta[getBuildMeta: read engine & version from DB]
    CheckIncremental -->|No| Parse[Parse files]
    GetMeta --> Compare{Engine or version changed?}
    Compare -->|Yes| Warn[Warn: Consider --no-incremental]
    Compare -->|No| Parse
    Warn --> Parse
    Parse --> BuildGraph[Build nodes & edges]
    BuildGraph --> SetMeta[setBuildMeta: persist engine, version, timestamp]
    SetMeta --> Close[Close DB]
    
    subgraph "Config Processing"
        LoadConfig --> CheckTop{Top-level excludeTests exists?}
        CheckTop -->|Yes| CheckNested{Nested query.excludeTests exists?}
        CheckTop -->|No| UseDefaults[Use defaults]
        CheckNested -->|Yes| UseNested[Use nested value]
        CheckNested -->|No| Hoist[Hoist to query.excludeTests]
        Hoist --> Cleanup[Delete top-level property]
        UseNested --> Cleanup
        Cleanup --> Done[Return merged config]
    end
Loading

Last reviewed commit: f65b364

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@carlos-alm carlos-alm merged commit f7f450b into main Feb 26, 2026
17 of 18 checks passed
@carlos-alm carlos-alm deleted the feat/build-metadata-excludetests branch February 26, 2026 04:19
@carlos-alm
Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in 8e08725:

setBuildMeta failure logging (line 958): Upgraded from debug to warn — metadata persistence failures may indicate disk issues and should be visible to users, not hidden behind debug-level logging.

All 565 tests pass.

@greptileai

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant