Silong/fc2 eval multi region

[silongtan]

Summary by CodeRabbit

• New Features

  • Introduced file-based recall metrics and file cardinality tracking to improve code location evaluation accuracy.
  • Enhanced reporting with multi-region decision analysis, including file-pattern coverage metrics and detailed diagnostics in verbose output.

• Tests

  • Added multi-region decision test fixtures with comprehensive evaluation scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes update the code locator evaluation system to support multi-region decisions by adding new metrics (recall_at_files, file_cardinality, multi_region), updating database path selection logic with fallback handling, and extending test fixtures with five multi-region decision cases for comprehensive testing.

Changes

Cohort / File(s)	Summary
Evaluation Metrics & Logic tests/eval_code_locator.py	Updates get_adapter() with fallback DB path selection (./.bicameral/local/code-graph.db → ./.bicameral/code-graph.db). Adds per-decision metrics during evaluation: recall_at_files, file_cardinality, and multi_region. Extends verbose output to annotate multi-region decisions and display new metrics. Aggregate reporting now includes avg_recall_at_files, multi_region_count, multi_region_recall_at_files, and file_cardinality_distribution.
Test Fixtures tests/fixtures/expected/decisions.py	Introduces BICAMERAL_MULTI_REGION fixture list with five FC-2 multi-region decision entries (ingest/grounding, multi-channel retrieval with RRF fusion, drift detection, team dual-write/event materialization, coverage-tier threshold broadening). Updates ALL_DECISIONS to include new multi-region fixtures and adds MULTI_REGION as filtered subset.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• decision grounding reuse + coverage loop #5: The evaluation changes directly extend grounding-focused functionality introduced in this PR, adding region-grounded metrics and multi-region decision support.

Suggested reviewers

• jinhongkuan

Poem

🐰 Hops through regions, metrics bloom,
Recall at files clears the gloom,
Multi-region dreams take flight,
Fixtures shine with test-case light,
Fallback paths, so smooth and spry,
Evaluation reaches for the sky! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Silong/fc2 eval multi region' is vague and lacks clarity. It uses branch-naming conventions as a title and doesn't clearly convey the main purpose of the changes (adding multi-region evaluation metrics and fixtures).	Consider revising the title to be more descriptive, e.g., 'Add multi-region evaluation metrics and fixtures' or 'Implement multi-region decision support in code locator evaluation'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch silong/fc2-eval-multi-region

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 (1)

tests/fixtures/expected/decisions.py (1)

499-509: ⚠️ Potential issue | 🟠 Major

Keep the FC-2 fixtures out of the global registry.

tests/eval_code_locator.py only repo-filters when more than one repo is passed. With this addition, any single-repo run against a non-bicameral checkout will now execute these bicameral-only cases and skew the reported metrics. Prefer wiring BICAMERAL_MULTI_REGION through an explicit FC-2 subset, or fix the caller to always repo-filter.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/fixtures/expected/decisions.py` around lines 499 - 509, ALL_DECISIONS
currently includes BICAMERAL_MULTI_REGION which causes bicameral-only FC-2 cases
to be run for single-repo test runs; remove BICAMERAL_MULTI_REGION from the
ALL_DECISIONS tuple and instead expose it only via an explicit FC-2 subset
(e.g., create FC2_BICAMERAL_DECISIONS or add BICAMERAL_MULTI_REGION to an
existing FC2_DECISIONS collection) so that callers can opt-in to bicameral
cases; update any test harness assembly that builds the FC-2 test set to include
this new subset when appropriate (refer to the ALL_DECISIONS and
BICAMERAL_MULTI_REGION symbols to locate the change).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/eval_code_locator.py`:
- Around line 231-237: The multi-region aggregates drop errored FC-2 cases
because mr_results is built only from entries where r.get("multi_region") is
truthy and the exception branch doesn't append a multi_region entry; update the
exception handling in the block that builds results (the code that currently
appends precision/recall/mrr on success) to also append a result dict with
"multi_region": True and zeros for "precision", "recall", "mrr" and
"recall_at_files" when ground_mappings() raises, so mr_results, mr_count and
mr_recall_at_files correctly include errored multi-region cases; apply the same
change to the analogous aggregation block that starts around where the other
multi-region aggregates are computed (the block referenced in the comment for
lines 261-263).

---

Outside diff comments:
In `@tests/fixtures/expected/decisions.py`:
- Around line 499-509: ALL_DECISIONS currently includes BICAMERAL_MULTI_REGION
which causes bicameral-only FC-2 cases to be run for single-repo test runs;
remove BICAMERAL_MULTI_REGION from the ALL_DECISIONS tuple and instead expose it
only via an explicit FC-2 subset (e.g., create FC2_BICAMERAL_DECISIONS or add
BICAMERAL_MULTI_REGION to an existing FC2_DECISIONS collection) so that callers
can opt-in to bicameral cases; update any test harness assembly that builds the
FC-2 test set to include this new subset when appropriate (refer to the
ALL_DECISIONS and BICAMERAL_MULTI_REGION symbols to locate the change).

🪄 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: 1327be0c-935b-44ad-b600-1afd00a3d248

📥 Commits

Reviewing files that changed from the base of the PR and between cf37eaf and 2e9c47f.

📒 Files selected for processing (2)

• tests/eval_code_locator.py
• tests/fixtures/expected/decisions.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Count errored multi-region cases in these aggregates.

mr_results is filtered on r.get("multi_region"), but the exception branch only appends precision/recall/mrr. If ground_mappings() raises on an FC-2 case, it drops out of multi_region_count and multi_region_recall_at_files instead of contributing zero.

Proposed fix

         except Exception as e:
             results.append({
                 "description": d["description"][:80],
                 "query": query,
                 "error": str(e),
                 "precision": 0, "recall": 0, "mrr": 0,
+                "recall_at_files": 0,
+                "file_cardinality": 0,
+                "multi_region": d.get("multi_region", False),
             })
             continue

Also applies to: 261-263

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval_code_locator.py` around lines 231 - 237, The multi-region
aggregates drop errored FC-2 cases because mr_results is built only from entries
where r.get("multi_region") is truthy and the exception branch doesn't append a
multi_region entry; update the exception handling in the block that builds
results (the code that currently appends precision/recall/mrr on success) to
also append a result dict with "multi_region": True and zeros for "precision",
"recall", "mrr" and "recall_at_files" when ground_mappings() raises, so
mr_results, mr_count and mr_recall_at_files correctly include errored
multi-region cases; apply the same change to the analogous aggregation block
that starts around where the other multi-region aggregates are computed (the
block referenced in the comment for lines 261-263).