witness gate: the near-miss regex flags ordinary prose, and its file-level exemption swallows genuine de-marked markers in the gate's own file - #344
Conversation
Tighten the near-miss detector to require the :: marker payload so ordinary prose that merely mentions WITNESS is no longer flagged, while de-marked (zero-width) and malformed markers still are. Replace the file-level de-marked-marker exemption with a line-level one keyed on the three documented docstring example lines (4, 19, 36), so a genuine de-marked marker appended to scripts/check_witness_token.py is reported. Adds regression tests and a changelog fragment.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe witness gate now requires a ChangesWitness gate hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change tightens witness-marker detection and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| # zero-width character (e.g. U+200B) lodged between WITNESS and the colon, | ||
| # or the colon replaced. Requiring the ``::`` payload keeps ordinary prose | ||
| # mentioning WITNESS from being mistaken for a malformed marker. | ||
| _NEAR_MISS_RE = re.compile(r"#\s*WITNESS[^:](?=.*::)") |
There was a problem hiding this comment.
WARNING: Near-miss regex can false-positive on prose containing ::
The (?=.*::) lookahead matches :: anywhere on the line. A prose comment like # WITNESS markers use :: syntax would be flagged as a near-miss even though it is ordinary prose, because the regex only checks that :: exists somewhere after the broken separator, not that it appears in a marker-like payload position.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # Documented examples of a de-marked marker in this file's own docstring. | ||
| # They are intentionally de-marked and must not be reported; every other line | ||
| # in the same file (e.g. an appended genuine marker) still is. Greppable name. | ||
| _DEMARKED_MARKER_EXEMPTION = { |
There was a problem hiding this comment.
SUGGESTION: Hardcoded exemption line numbers are fragile
The docstring example lines (4, 19, 36) are hardcoded. If the docstring is edited, these line numbers become stale and the exemption silently breaks. Consider matching by line content (e.g., the specific de-marked marker pattern) instead of position.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # that merely mentions WITNESS, while still catching de-marked (ZWSP) | ||
| # and malformed markers. All three arms live in one test so a loosening | ||
| # that silences the prose cannot silence the de-marked arms either. | ||
| # Arm A -- prose in taosmd/ mentioning WITNESS but no ``::`` is clean. |
There was a problem hiding this comment.
SUGGESTION: Missing test arm for prose containing ::
Arm A covers prose without ::, but there is no arm for prose that mentions both WITNESS and :: in the same line (e.g., # WITNESS markers use :: syntax). The current regex would flag such a line as a near-miss. Consider adding an Arm D to verify this edge case.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 59.9K · Output: 19.9K · Cached: 640.9K |
MERGE. Both card items delivered and measured, with one regression carded as
|
…inside methods (#350) Revision of #343. Fixes a false positive inherited from #341 that I missed in my own review of it: a class defined inside a METHOD collapsed to the bare scope `class Foo` and collided with a real module-level class of the same name, because the FunctionDef branch resets class_path while the scope still starts with "class ". Fix is the one-line guard `if scope == "module" or " > " not in scope:`. 16 scope shapes, each measured against master, the pre-fix rebase commit e6b270a, and this HEAD: SHAPE master e6b270a PR350 want CTL module fn dup FIRE FIRE FIRE FIRE CTL method dup in one class FIRE FIRE FIRE FIRE CTL no dup at all silent silent silent silent CTL same method name, 2 module classes silent silent silent silent class-in-METHOD vs module class silent *FIRE* silent silent class-in-METHOD, dup method inside silent FIRE FIRE FIRE same class-in-method in 2 methods silent *FIRE* silent silent class-in-CLOSURE vs module class silent silent silent silent class-in-closure dup method silent FIRE FIRE FIRE same class-in-closure, 2 closures silent silent silent silent nested class, same method name silent silent silent silent nested class dup method FIRE FIRE FIRE FIRE closure fn dup in one parent FIRE FIRE FIRE FIRE same closure fn, 2 parents silent silent silent silent if/else sibling arms silent silent silent silent try/except ImportError fallback silent silent silent silent mismatches: 2 2 0 master misses two real duplicates; e6b270a gains those and adds two false positives; this HEAD keeps both gains with zero mismatches. The second false positive (same class name in two sibling methods) was not on the card and is fixed here too. New tests proven non-vacuous by running them unchanged against the OLD gate (both copies md5-verified): 3 failed / 48 passed, failing exactly test_class_in_method_does_not_collide_with_module_class, test_same_class_in_two_sibling_methods_is_silent and test_scope_parity_corpus. Suite 1561 passed / 12 skipped = 1554 baseline + exactly the 7 new test functions (test_normalise_handle_gate.py 44 -> 51). Re-measured on the new master after my own #344 merge moved it. All three gates clean. Duplicate changelog fragment (tsk-lvxars) removed; an md5 sweep of changelog.d finds no byte-identical pair. ImportError docstring sentence restored byte-exact. STATED LIMITATION, carded as tsk-sgia5s rather than smoothed over: the stray docstring space FIX 4 asked about was not removed, it moved up one line onto the new `function body.` continuation. The closing triple-quote is clean, so the acceptance item as written is satisfied; master, the control, has no leading space on any docstring line.
1 participant
CARD TITLE (intent, not commit subject): witness gate: the near-miss regex flags ordinary prose, and its file-level exemption swallows genuine de-marked markers in the gate's own file
Autonomous build of board card tsk-2k55kq.
Tighten the near-miss detector to require the :: marker payload so
ordinary prose that merely mentions WITNESS is no longer flagged, while
de-marked (zero-width) and malformed markers still are. Replace the
file-level de-marked-marker exemption with a line-level one keyed on the
three documented docstring example lines (4, 19, 36), so a genuine
de-marked marker appended to scripts/check_witness_token.py is reported.
Adds regression tests and a changelog fragment.
Files:
.../tsk-2k55kq-witness-gate-near-miss-hardening.md | 8 +++
scripts/check_witness_token.py | 21 ++++---
scripts/resume_arm_time.py | 8 ++-
tests/test_resume_arm_time.py | 8 ++-
tests/test_witness_gate.py | 67 ++++++++++++++++++++++
5 files changed, 99 insertions(+), 13 deletions(-)
Summary by CodeRabbit
Bug Fixes
Tests