Repository navigation
fix: preserve Markdown code spans in guidance checks - #477
Conversation
User evaluation at e2eae98: ran the actual guidance guard against literal Markdown fixture trees. Matching double and triple code spans, including a completed earlier span, return 0; an invalid Manifest parity, desired-state digests, bundled skill specification validation, ShellCheck and release gates pass locally. CI and a successful current-head review must still settle before promotion. |
User evaluation at af13476: the actual repository guard reports 33 field lists across all 162 surfaces with no invalid field. Matching double/triple spans and multiline command spans pass; invalid Independent review drove repairs for escaped prose, container fences and indentation, paragraph/heading boundaries and HTML comments. The observer retains its position across commands; the 10,000-command literal guidance test passes with roughly linear benchmark scaling. Manifest parity, digests, skill specification validation, ShellCheck and release gates pass locally. Hosted CI and a qualifying review must still settle before promotion. |
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🔇 Additional comments (5)
📝 WalkthroughWalkthroughThe change adds Markdown delimiter tracking to shell-field normalization. The observer recognizes inline backtick runs, fences, block boundaries, comments, escapes, and quote or list containers. The normalizer uses matching inline delimiters instead of checking backtick parity from the current line. New Go and guard tests cover Markdown spans, boundaries, unresolved shell syntax, and repeated-span benchmarks. Priority: ⬇️ Low Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to No confirmed issue blocks merging, subject to the outstanding CI checks. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
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 |
The explicit Docstring Coverage finding in CodeRabbit’s review is addressed at f59ba37. All seven touched named Go functions now have attached documentation comments (three production functions and four tests/benchmarks); the benchmark callback also has an explanation. Independent Go parsing verified the docgroups. The repair adds only five comment lines. The runtime observer blob is unchanged from the CodeRabbit-reviewed head af13476, whose 45 CI checks passed and whose substantive review reported no actionable code findings. Current-head CI and the required review route remain pending before promotion. |
devantler
left a comment
devantler
left a comment
There was a problem hiding this comment.
🤖 Generated by the Agentic Engineer
Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)
Reviewed commit: f59ba37
- CodeRabbit — authenticated
coderabbitai[bot]review summary, provider event 2026-10-04T18:51:06Z, freshly observed at 2026-10-04T19:03:25Z: this PR's completed review consumed its included allowance; the plan allows one included review per hour and reports zero remaining. The earlier request was at 18:46:04Z, so the ongoing hourly limit applies to this new head. Recovery is the included allowance refill; no paid retry is authorized. That substantive review covers the unchanged runtime at af13476, but does not satisfy this new head's review gate. - Codex — authenticated
chatgpt-codex-connector[bot]account usage refusal, provider event 2026-10-04T02:11:02Z, freshly observed at 2026-10-04T19:03:25Z: the account has reached its code review usage limit. No reset time is stated. Recovery requires renewed included allowance or account action; paid credits are not authorized. - Cursor Bugbot — authenticated
cursor[bot]usage refusal, provider event 2026-10-04T12:21:20Z, freshly observed at 2026-10-04T19:03:25Z: this user or team's usage/spend limit prevented the review. No reset time is stated. Recovery requires allowance renewal or administrative action; raising the spend limit is not authorized.
Direct current-PR reads cover all conversation comments, review objects and review threads with complete pagination, plus the native current-head check list. There is no current-head external review, no Bugbot run, no outstanding thread and no newer provider recovery artifact on this PR. Disabled automatic review is uninformative and is not the evidence for fallback.
I reviewed the complete three-file diff against base 59c8266 for correctness, execution safety, retained-value boundaries and regression coverage. Matching backtick runs preserve literal single-, double- and triple-delimited field lists, including completed earlier spans and spans across a nonblank newline. Escaped prose, code fences, container-relative indentation, headings, paragraph breaks and HTML comments cannot supply a misleading opener. The observer advances monotonically rather than rescanning the full prefix for every command.
The normalizer does not execute inspected commands or source packages. Unknown expansions, mismatched delimiters and unresolved shell expressions remain UNKNOWN; a literal invalid merged field remains a failure. Tests cover positive guidance and adversarial contexts, including heading starts and decoded % boundaries. The supported guidance syntax is bounded, not a general Markdown or shell interpreter. An unterminated HTML comment can conservatively cause a later false UNKNOWN, never a false clean result; that limitation does not widen execution or trust.
Validation includes 128 passing CLI guard regressions, the complete Go parser tests, ShellCheck and the actual repository scan: 33 field lists across 162 surfaces. The repeated-span test checks all 10,000 spans, and 1,000/10,000-span benchmarks showed roughly linear scaling. The independent final diff audit found no actionable defect.
The prior review's explicit Docstring Coverage finding is addressed in the current head: all seven touched named Go functions have attached documentation, independently verified with Go AST parsing, and the benchmark callback has an explanatory comment. The final commit adds only five comment lines; runtime blob a671f0d6c72d4e3d6bf0f9b8c2f4bf3676641c2f is unchanged from CodeRabbit's reviewed head. This is source verification, not a claim that the provider reran its ancillary evaluator. The resolution record records the repair.
No outstanding correctness, security or documentation finding remains at this head.
Verdict: no P0/P1 findings
Ready at f59ba37 against base 59c8266. The native current-head CI read reports 45 settled checks, including successful The substantive current-head local review satisfies the canonical local-review gate. It records freshly authenticated, applicable limits for all three providers and reviews the full diff; no additional paid request was made. CodeRabbit's disabled-auto-review status is uninformative and is not being treated as a review. User evaluation exercised the actual guard: 128 CLI cases pass, the Go suite passes including the final heading-start regression, and the current-head repository scan checks 33 field lists across 162 surfaces. Valid literal Markdown guidance passes; invalid fields fail; unresolved syntax and unrelated block openers remain UNKNOWN. The final comments-only repair addresses the explicit documentation finding and leaves the evaluated runtime unchanged. No inspected commands or packages execute. All three readiness conditions are met. Promoting and merging with an exact head match. |
Delivered: PR #477 merged at 2026-10-04T19:06:28Z as c440fb3. A fresh full-ref fetch confirms that commit is on I ran the actual merged tree in an isolated archive: all 128 CLI guard cases pass, the complete Go parser suite passes, and the shipped repository scan reports 33 field lists across 162 surfaces with no invalid field. This confirms the Markdown compatibility repair and the conservative UNKNOWN behavior on unresolved syntax after merge. The documentation repair is included in the same delivered tree. Issue #476 is closed and verified ✅ Done on Project 5. The round's 13 delivery issues are all closed/Done, and live open-PR inventories for both libraries are empty. |
1 participant
Why
Valid commands inside double-backtick Markdown spans now fail the guidance check. This prevents existing documentation from passing validation and makes the checker report an incomplete observation for a literal command it can safely inspect.
What
Match Markdown opening and closing delimiter runs by length, preserving spans across line breaks. Escaped prose, separate blocks and retained decoded values cannot supply a misleading opener. Track fenced and indented code inside ordinary, quoted and list examples. Observe the source once across successive commands, keep unresolved shell expressions UNKNOWN, and continue rejecting invalid GitHub JSON fields inside valid code spans.
Validation: added regressions reproduced the delimiter and context failures before each repair; the guard now passes all 128 CLI cases. Go tests cover separate-block and decoded-value boundaries, 10,000 literal command spans and a benchmark. ShellCheck, the actual 162-surface repository scan, manifest parity, desired-state digests, all bundled skill specification checks and release gates pass. Scanned commands are never executed. The observer supports bounded guidance syntax and does not claim full Markdown or shell interpretation.
Fixes #476
Follow-up to #473.