fix(visual): filter a vision finding's model-authored path through toPublicSafe

[RealDiligent]

Summary

VisualVisionFinding's JSDoc (src/review/visual/visual-findings.ts:124) documents both path and body as already public-safe, but parseVisualVisionResponse only ran body through toPublicSafe — path got a bare .trim(). path is model-authored free text off the vision response JSON (nothing validates it is a real route, and findVisualEvidence just returns undefined on no match, so an arbitrary string still produces a finding), and it is interpolated verbatim into the finding's public title (Possible visual regression: ${finding.path}).

The two title consumers disagree: unified-comment-bridge.ts re-scrubs it, but visual-followup.ts does not — it cites the (false) JSDoc. So injected markdown / @mention markup in a model path reached a public follow-up comment live.

Fix

• parseVisualVisionResponse runs path through toPublicSafe exactly as it does body, dropping the entry when it returns null (the existing if (!path || !body) continue; shape).
• A new MAX_VISUAL_PATH_CHARS constant (beside MAX_VISUAL_FINDINGS) bounds the filtered path so one finding's title cannot flood a public comment.
• An ordinary route path (/app, /docs/getting-started) survives toPublicSafe byte-identically, so findVisualEvidence's route.path === path lookup still matches — pinned by a test that asserts both the byte-identical round-trip and that visualEvidence still attaches.

body's filtering, the MAX_VISUAL_FINDINGS cap, the permissive category parse, the []-on-unparseable fail-safe, the two finding codes/severity, and visualFindingsFromFindings's defence-in-depth re-scrub are all unchanged; the JSDoc contract is now true rather than edited to match the old behaviour.

Tests

• visual-findings.test.ts: a path with markdown/@mention markup yields a finding whose path no longer contains the live link markup; a path longer than the constant is truncated to it; an ordinary route path round-trips byte-identically and still attaches visualEvidence for a captured route.
• visual-followup.test.ts: a follow-up comment built from a hostile model path does not contain the injected live link.

The scrub/truncate/followup cases fail against the current bare-.trim() code. Coverage of the changed file is 100% (statements/branches/lines).

Closes #10062

[loopover-orb]

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-31 07:07:39 UTC}

3 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This fix correctly closes the gap where a model-authored vision `path` was only `.trim()`ed while `body` went through `toPublicSafe`, allowing markdown/@​mention injection into the public follow-up comment title via `visual-followup.ts`. The change applies `toPublicSafe` to `path` symmetrically with `body`, drops the entry on a null result via the existing `if (!filteredPath || !body) continue;` guard, and adds a `MAX_VISUAL_PATH_CHARS` bound. Tests verify the markup is stripped, the length is bounded, and — importantly — that an ordinary route path round-trips byte-identically so `findVisualEvidence`'s exact-match lookup still attaches evidence.

Nits — 3 non-blocking

• src/review/visual/visual-findings.ts: the `MAX_VISUAL_PATH_CHARS` truncation happens after `toPublicSafe` filtering rather than before, so a very long hostile path still pays the full `toPublicSafe` scan cost — likely negligible given `MAX_VISUAL_FINDINGS` caps at 3 entries, but worth confirming there's no quadratic cost in `toPublicSafe` itself for large inputs.
• test/unit/visual-findings.test.ts: the truncation test only checks `finding!.path.length).toBe(256)` and doesn't assert the truncated path no longer round-trips into `findVisualEvidence`, though the intent (truncated paths simply won't match any real route) is covered indirectly by the round-trip test using a short path instead.
• Consider adding one negative-space test: a path over 256 chars combined with a real matching route of that exact (unrealistic) length, to make explicit that truncation intentionally breaks the route match rather than relying on real routes always being short.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#10062
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	❌ 8/20	High review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 239 registered-repo PR(s), 89 merged, 38 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	RealDiligent; Gittensor profile; 239 PR(s), 38 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Linked issue satisfaction

Addressed
The diff applies toPublicSafe to record.path exactly as it does to body, drops the entry on null (preserving the existing if-check shape), adds a MAX_VISUAL_PATH_CHARS constant beside MAX_VISUAL_FINDINGS to bound the scrubbed path, and includes tests confirming an ordinary route path round-trips unchanged so findVisualEvidence's route.path === path lookup still matches, while leaving visual-follow

Review context

• Author: RealDiligent
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 239 PR(s), 38 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Add a concise scope and risk note.
• Then work through the remaining 1 step in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.59%. Comparing base (a7673e2) to head (5dd18c3).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #10101       +/-   ##
===========================================
- Coverage   91.88%   79.59%   -12.29%     
===========================================
  Files         930      283      -647     
  Lines      113827    58720    -55107     
  Branches    27466     8665    -18801     
===========================================
- Hits       104588    46738    -57850     
- Misses       7940    11694     +3754     
+ Partials     1299      288     -1011     

Flag	Coverage Δ
backend	100.00% <100.00%> (+4.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/review/visual/visual-findings.ts	100.00% <100.00%> (ø)

... and 780 files with indirect coverage changes

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.