Skip to content

review: fix-handoff-render's markdownPathCodeText backslash-escapes backticks, which markdown code spans don't honor #9289

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

markdownPathCodeText in src/review/fix-handoff-render.ts (lines ~43-49) attempts to make a
finding's path safe for inline markdown code spans:

function markdownPathCodeText(value: string): string {
  return value
    .replace(/\\/g, "\\\\")
    .replace(/`/g, "\\`")
    .replace(/\|/g, "\\|")
    .replace(/[<>]/g, (char) => (char === "<" ? "&lt;" : "&gt;"));
}

Its callers (buildFixHandoffBlock at line ~57 and fixHandoffAggregateItem at line ~108) wrap
the result in a literal single-backtick code span (`${location}`). CommonMark/GFM code spans
are delimited by a run of backticks, not affected by a preceding backslash — backslash-escaping a
backtick does not prevent it from prematurely closing the span. So a finding.path containing
a literal backtick character still corrupts the rendered fix-handoff block.

The correct fix already exists twice in the sibling file src/review/unified-comment-bridge.ts:
markdownPathCode (line ~722) and markdownChangedFilePath (line ~624) both compute a code-span
delimiter longer than any backtick run found inside the value, instead of trying to escape
backticks. markdownPathCode's own doc comment states the reason explicitly: "choose a code-span
delimiter longer than any run inside the value instead of trying to backslash-escape backticks
(Markdown does not honor that inside code spans)."

test/unit/fix-handoff-render.test.ts never exercises a path containing a backtick, so this gap
is untested.

Requirements

  • markdownPathCodeText in src/review/fix-handoff-render.ts must use the same
    delimiter-length approach as markdownPathCode in src/review/unified-comment-bridge.ts
    (compute the longest run of backticks in the value, use a delimiter one longer, do not
    backslash-escape backticks) rather than the current backslash-escape approach.
  • Its callers (buildFixHandoffBlock, fixHandoffAggregateItem) currently wrap the result in a
    fixed single-backtick span — since the fixed function now returns its own delimiter-wrapped code
    span (matching markdownPathCode's return shape, which includes the delimiters and surrounding
    spaces), update both call sites to use the function's own delimiters directly instead of
    re-wrapping in a hardcoded single backtick, mirroring how unified-comment-bridge.ts's callers
    consume markdownPathCode's return value.
  • Preserve the existing entity-escaping behavior for |, <, > — only the backtick-handling
    strategy changes.
  • Do not modify src/review/unified-comment-bridge.ts — it is already correct and is the
    precedent to copy from, not to change.

Deliverables

  • markdownPathCodeText in src/review/fix-handoff-render.ts rewritten to use a
    longest-backtick-run-plus-one delimiter instead of backslash-escaping, matching
    markdownPathCode's approach in src/review/unified-comment-bridge.ts.
  • buildFixHandoffBlock and fixHandoffAggregateItem updated so the rendered location string
    uses the function's own delimiter-wrapped output rather than a hardcoded single-backtick
    wrap around it.
  • test/unit/fix-handoff-render.test.ts gains a test case with a finding.path containing a
    literal backtick, asserting the rendered fix-handoff block's code span is not corrupted
    (i.e. the backtick-containing path renders as a single, unbroken code span using a longer
    delimiter, not a broken span).

All three deliverables are required in this single PR.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted, on every changed line/branch in
src/**. The rewritten markdownPathCodeText and both updated call sites must be covered by the
new backtick-path test case above, plus existing tests must continue passing for
non-backtick-containing paths.

Expected Outcome

A finding path containing a backtick character renders as a correct, unbroken markdown code span
in the fix-handoff block, matching the behavior already correct in
src/review/unified-comment-bridge.ts's equivalent renderer.

Links & Resources

  • src/review/fix-handoff-render.ts (function and call sites to fix)
  • src/review/unified-comment-bridge.ts:722 markdownPathCode (precedent — required pattern to
    mirror, including its doc comment explaining why escaping doesn't work)
  • src/review/unified-comment-bridge.ts:624 markdownChangedFilePath (second precedent example)
  • test/unit/fix-handoff-render.test.ts (test file to extend)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions