Repository navigation
fix(context): count a traced pair's hotness once per caller - #3533
Conversation
Trace confirmation writes a pair's observation onto each of its static sites, so the hotness query returns one row per call site, each with the pair's whole dynamic_call_count. _hotness summed them, and a caller with two call sites to the target read as twice as hot as the trace observed. It now takes the count once per caller. Closes #3532
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 25 minutes. View limit details
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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 [Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryFixes caller ranking in
Reviews (1) · Last reviewed commit: "fix(context): count a traced pair's hotn..." · Reviewed by Greptile |
|
Summary
cgr contextand the MCPcontexttool break ties among a target's callers by trace hotness,dynamic_call_count. The hotness query returns one row per call site, and trace confirmation writes a pair's whole observation onto each of its sites ([Bug]: any incremental re-parse of a file silently discards its trace-derived (dynamic) CALLS edges; dead-code flips back after a comment edit #2429)._hotnesssummed those rows, so a caller with two call sites to the target read as twice as hot as the trace observed, and could outrank a caller the trace saw more often. It now takes the count once per caller.Demo
No change on the default embedded engine today: there the observation currently lands on one site of the pair, so the sum and the single count agree. On Memgraph, every site carries it, so a caller with several sites was ranked by the inflated sum. #3529 brings the embedded engine to every site as well. The new test shows the counts.
Type of Change
Related Issues
Closes #3532
Test Plan
make test-paralleloruv run pytest -n auto -m "not integration")make test-integration, requires Docker)Red first:
test_hotness_counts_a_traced_pair_once_however_many_sites_it_hasfailed onmain, withrun(two sites, observed 5 times) at 10. Green: 32 passed acrosstest_context_slice.py,test_context_source_root.pyandtest_context_slice_normalisation.py.Negative test: in that same test, two call sites do not make
run(5) hotter thanother, which the trace observed 7 times.Checklist
!in the title and its release notes inchanges/breaking/<PR number>.md(see Breaking changes)make pre-commit)# type: ignore,cast(),Any, orobjecttype hints