fix(#69): skip tests of removed preflight contracts (12f25eb)#82
Conversation
Issue BicameralAI#69: ``tests/test_v055_region_anchored_preflight.py`` and ``tests/test_v0412_preflight.py`` failed to collect with: ImportError: cannot import name '_merge_decision_matches' from 'handlers.preflight' ImportError: cannot import name '_has_actionable_signal_in_search' from 'handlers.preflight' Both helpers were removed in commit 12f25eb (v0.10.0 — hierarchical dashboard, history-based preflight, per-section ingest). The preflight refactor dropped BM25 topic search entirely; preflight now reads ``bicameral.history()`` and uses LLM reasoning to identify relevant feature groups. The contracts these test files exercised no longer exist on the new code path. Fix: - Module-level ``pytest.skip(... allow_module_level=True)`` on both files with a reason that points at the v0.10.0 commit and explains what was removed. - The original imports are kept (with ``# noqa: E402, F401``) below the skip — they document the test files' original surface area for future port-forward work, but are unreachable. - The module docstrings are extended with a "Status (issue BicameralAI#69)" paragraph so anyone opening the file sees why it's quarantined before they look for the imports. Note: ``tests/test_extraction_metrics.py`` was also listed in BicameralAI#69 but collects fine on this branch (16 tests). It is left untouched. Tests: ``pytest`` collects both files cleanly (0 errors); each shows as 1 ``skipped`` with a readable reason.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
1 participant
Closes #69.
Summary
Two test files failed to collect on
main:tests/test_v055_region_anchored_preflight.py— imports_merge_decision_matchesfromhandlers.preflighttests/test_v0412_preflight.py— imports_has_actionable_signal_in_searchfromhandlers.preflightBoth helpers were removed in commit 12f25eb (v0.10.0 — hierarchical dashboard, history-based preflight, per-section ingest). The preflight refactor dropped BM25 topic search entirely; preflight now reads
bicameral.history()and uses LLM reasoning to identify relevant feature groups. The contracts these test files exercised no longer exist on the new code path.Fix
Module-level
pytest.skip(... allow_module_level=True)on both files with a reason that points at the v0.10.0 commit and explains what was removed.The original imports are kept (with
# noqa: E402, F401) below the skip — they document the original surface area for future port-forward work, but are unreachable. The module docstrings are extended with a "Status (issue #69)" paragraph so anyone opening the file sees why it's quarantined before they go looking for the imports.Why skip rather than delete?
The scenarios documented in these files (region-anchored decision retrieval, "actionable signal" detection on search responses) informed the v0.10.0 redesign. Keeping the files preserves that history; deleting them would lose the design rationale for anyone reading git log later. If/when the maintainers prefer outright removal, this is a one-commit cleanup.
Why not port forward?
Three of the v0.4.12 helpers (
_validate_topic,_dedup_key_for,_check_dedup) still exist on the new code path, so a partial port is possible — but the handler-level mock tests are tied to the old BM25 pipeline and would need full rewrites against the LLM-reasoning-based preflight. That's a separate, scoped refactor; out of band for this unblocking fix.Note on
tests/test_extraction_metrics.pyIssue #69 also listed
test_extraction_metrics.pyas failing collection. On the currentmain(6bdff24) it collects fine (16 tests). The file is left untouched.Verification
pytest tests/test_v055_region_anchored_preflight.py tests/test_v0412_preflight.py --collect-only -q— 0 collection errors, 2 skipped with reasonstests/Test plan