normalise-handle gate: the ImportError special case is dead code, its three tests cannot fail, and the widening changelog was deleted - #340
Conversation
…ts, restore widening changelog - Remove the in_import_error_except flag and _is_import_error_handler, threaded through eleven call sites but never read by _duplicate_definitions; the sibling-arm rule now handles the ImportError/ModuleNotFoundError fallback silence - Give the three spelling tests and the scope-parity corpus fallback cases real duplicate definitions (def in try body plus def in handler) so they exercise the fallback silence, proven RED against tsk-bpadgg gate - Restore changelog.d/tsk-xhvo4a-duplicate-definition-gate-scope.md, deleted in tsk-2ohqj4 but describing widening that still ships
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 27 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Reviewed by step-3.7-flash · Input: 59.5K · Output: 8.7K · Cached: 240.3K |
|
MERGE. All seven acceptance items on Acceptance, measured
The vacuity claim is confirmed, and the fix discriminatesGate versions loaded by path, The old fixtures are silent on #328's own gate, which is the gate they were nominally guarding against. The card's claim that they had zero power is exact. Caveat on acceptance 3, and a correction to the PR bodyThe body says the spelling tests and "the scope-parity corpus fallback cases" are "proven RED against tsk-bpadgg gate". That holds for four of the five rewritten fixtures. It does not hold for This is not a blocker. Acceptance 3 asked only that the corpus case contain a real duplicate, and it does — the fixture is a genuine two-arm duplicate rather than the old Acceptance 7: the count the body omitsMeasured here on a trial merge of Identical to master's baseline of 1552/12, which reconciles exactly against what the diff touched: three tests rewritten in place, none added, none removed. The card's stated baseline of "1539 passed, 12 skipped" is stale — master has moved through #337 and #338 since the card was filed. Gates on the trial merge, output read rather than exit code trusted: Conflict with #341, which is not this PR's fault#340 and #341 were dispatched from the same base |
…of/review_by coercion guards forward with tests (#347) Revision of the closed #312. All three blockers from tsk-agi5ph resolved, both non-blocking items addressed, and the tests verified red-first rather than taken on the body's word. RED FIRST, this PR's test files run UNCHANGED against #307 (swap proven by md5 first: test_api.py d2736eae and test_collections_ingest.py c5787bc1 identical on both sides; api.py ce0ac0e0 (#307) vs ccc68390 (#347) genuinely differs): the card's five-case table, on #307: FAILED coerce_iso_string_timestamp_returns_hits ValueError FAILED coerce_int_review_by_returns_hits TypeError '<' int/str FAILED coerce_list_review_by_returns_hits TypeError '<' list/str coerce_str_review_by_overdue_is_past_review passed coerce_plain_metadata_returns_hits passed 3 failed, 2 passed -- failure-for-failure identical to the card's prediction all six coercion tests on #307: 4 failed, 2 passed -- exactly the body's "4 of 6" claim BLOCKER 2, the dropped producer, restored rather than rebuilt: taosmd/collections.py is byte-identical to #307 (md5 744af0bb on both). The producer is genuinely absent on master -- the end-to-end test cannot even import there (ImportError: cannot import name '_parse_front_matter'), so it discriminates. indexed_at is written at collections.py:909. The PR body states the chosen direction explicitly, as acceptance required. BLOCKER 3: the false UnboundLocalError changelog entry is gone; replacement fragment ends 0x0a. Guards are now symmetric (api.py:486 as_of, api.py:494 review_by). Gates clean (deleted-symbols, normalise-handle, witness), no conflict markers, no Removes-Intentionally trailer. Suite 1617 passed, 12 skipped -- reconciles as the post-#351 baseline 1591 + the 26 tests added. The card's bar was >1435. STATED LIMITATIONS. - test_search_hit_metadata_always_has_doc_currency_fields PASSES on #307 too. It is a valid output-shape contract guard but is NOT evidence anything was fixed; the discriminating tests are the four coercion cases. - The red-first baseline is #307, not master: against master these tests cannot import at all, so master is useless as a red baseline for the producer half. - The PR body reports no suite count, which the card's acceptance asked for in as many words. Ran it here rather than bounce correct work. This is the sixth PR in that pattern (#340, #341, #345, #346, #348, #347) and is worth fixing at the source. - api.py:498 builds is_past_review in a 110-character expression. No line-length rule exists in the repo, so style only; no change requested.
1 participant
CARD TITLE (intent, not commit subject): normalise-handle gate: the ImportError special case is dead code, its three tests cannot fail, and the widening changelog was deleted
Autonomous build of board card tsk-w2ugkt.
threaded through eleven call sites but never read by _duplicate_definitions;
the sibling-arm rule now handles the ImportError/ModuleNotFoundError
fallback silence
real duplicate definitions (def in try body plus def in handler) so they
exercise the fallback silence, proven RED against tsk-bpadgg gate
in tsk-2ohqj4 but describing widening that still ships
Files:
.../tsk-xhvo4a-duplicate-definition-gate-scope.md | 2 ++
scripts/normalise_handle_gate.py | 36 ++--------------------
tests/test_normalise_handle_gate.py | 35 ++++++++++-----------
3 files changed, 21 insertions(+), 52 deletions(-)