Skip to content

fix(codegraph): correct BM25 IDF and transitive impact (#445) - #449

Open
whykusanagi wants to merge 1 commit into
mainfrom
fix/445-codegraph-impact
Open

whykusanagi wants to merge 1 commit into
mainfrom
fix/445-codegraph-impact

Conversation

@whykusanagi

@whykusanagi whykusanagi commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes #445: compute the documented smoothed BM25 IDF; walk up to three hops of callers, keyed by symbol ID rather than bare names, with stable traversal and deterministic caller ordering. Adds regression tests for the formula, invalid corpus frequencies, a three-hop chain, and duplicate-name callers.

Validation

go test ./... -count=1, Go-1.26.9 golangci-lint, CGO_ENABLED=0 go test ./cmd/celeste/codegraph, and Windows/Linux pure-Go codegraph cross-builds passed locally. full-ci is requested for macOS and Windows runtime tests.

This PR is only the codegraph WIP formerly held uncommitted on stabilize; it does not include the keychain scaffold or the outstanding #435 TUI model-hold change.

Summary by CodeRabbit

  • Improvements
    • Change impact analysis now includes callers up to three levels deep, helping surface more code that may be affected.
    • Distinct symbols with the same name are tracked separately, improving the accuracy of impact results.
    • Search relevance scoring now uses a smoothed calculation and handles invalid document-frequency counts safely.

@whykusanagi whykusanagi added the full-ci Run CI on macOS and Windows too label Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7e5c7f4d-04cb-4dc6-aacb-33517dc7dac2
📥 Commits

Reviewing files that changed from the base of the PR and between cb097b5 and 532f53f.

📒 Files selected for processing: 5
  • cmd/celeste/codegraph/bm25.go
  • cmd/celeste/codegraph/bm25_idf_formula_test.go
  • cmd/celeste/codegraph/changes.go
  • cmd/celeste/codegraph/changes_test.go
  • cmd/celeste/codegraph/store_snapshot.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The changes update BM25 IDF calculation and impact analysis. The IDF calculation uses a smoothed formula and rejects invalid corpus frequencies. Impact analysis distinguishes symbols by ID and traverses callers breadth-first for up to three levels.

Changes: BM25 IDF formula

Layer / File(s) Summary
Smoothed IDF calculation and validation
cmd/celeste/codegraph/bm25.go, cmd/celeste/codegraph/bm25_idf_formula_test.go
bm25Idf uses the smoothed formula and returns zero when the document frequency exceeds the corpus size. Tests cover valid formula results and invalid frequencies.

Transitive caller impact

Layer / File(s) Summary
Symbol IDs and caller lookup
cmd/celeste/codegraph/store_snapshot.go
SymbolsInLineRange now includes symbol IDs. The new CallersOfID method queries callers by target symbol ID.
Bounded caller traversal
cmd/celeste/codegraph/changes.go, cmd/celeste/codegraph/changes_test.go
AnalyzeChanges deduplicates changed symbols and callers by ID, then traverses callers breadth-first for up to three levels. Tests cover transitive callers and symbols with duplicate names.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant AnalyzeChanges
  participant Store
  participant CodegraphDB
  AnalyzeChanges->>Store: CallersOfID(targetID)
  Store->>CodegraphDB: Query callers connected by calls edges
  CodegraphDB-->>Store: Caller symbols with IDs
  Store-->>AnalyzeChanges: Caller symbols
  AnalyzeChanges->>AnalyzeChanges: Queue unseen callers up to depth three
Loading
Merge Risk: ⚪ Minimal · up to 532f5

This change corrects the IDF formula and adds transitive caller impact analysis with regression tests. No remaining merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Issue #445 has two coding requirements. bm25Idf now implements log((N-df+0.5)/(df+0.5)+1) and rejects invalid frequencies. AnalyzeChanges now performs a three-level, symbol-ID keyed breadth-firs… Preserve breadth-first depth order when producing AffectedCallers. Apply risk sorting only within the same traversal depth, or otherwise include depth as the primary ordering key. Add a regression test where a deeper caller has a higher r…
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely summarizes the two main changes: correcting BM25 IDF and fixing transitive codegraph impact analysis.
Out of Scope Changes check Passed The reviewed changes are limited to BM25 IDF handling, codegraph caller traversal, symbol-ID lookup, and related regression tests in cmd/celeste/codegraph. These changes support Issue #445. The evid…
Full details: Linked Issues check

Explanation

Issue #445 has two coding requirements. bm25Idf now implements log((N-df+0.5)/(df+0.5)+1) and rejects invalid frequencies. AnalyzeChanges now performs a three-level, symbol-ID keyed breadth-first walk, and the added tests cover the formula, invalid frequencies, a three-level chain, and duplicate names. However, affectedCallers is sorted by descending risk after traversal. A deeper caller with a higher risk can appear before a direct caller, so the required direct-callers-first ordering is not guaranteed.

Resolution

Preserve breadth-first depth order when producing AffectedCallers. Apply risk sorting only within the same traversal depth, or otherwise include depth as the primary ordering key. Add a regression test where a deeper caller has a higher risk than a direct caller.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@whykusanagi

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Labels

full-ci Run CI on macOS and Windows too

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codegraph: BM25 IDF deviates from documented formula; impact analysis is one-hop despite 'transitive' contract

1 participant