fix(review): guard hallucinated missing-symbol blockers + collapse nits - #1094
Conversation
…lapse nits Soak finding: the bot FALSE-CLOSED a good contributor PR (metagraphed #1528, green CI, mergeable) on a hallucinated blocker — it claimed loadArtifactData was 'not imported -> ReferenceError', but that function is defined locally and used by ~24 call sites. Root cause: the model sees only the DIFF, not the whole file, so new code referencing an existing (out-of-diff) symbol looks undefined; both free models made the same mistake -> consensus -> close. (Full-file grounding is enabled but did not prevent it, so a prompt-level guard is the reliable fix.) - DIFF SCOPE guard in the review prompt: never report a missing import / undefined symbol / 'X not defined -> ReferenceError' as a blocker unless the diff ITSELF removes the definition or introduces the symbol without defining it; otherwise it is at most a nit. Directly kills the false-close-on-hallucination class. - Nits now render inside a collapsed <details> toggle (assessment + blockers stay visible), per request — keeps the comment focused.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1094 +/- ##
=======================================
Coverage 94.77% 94.77%
=======================================
Files 153 153
Lines 18552 18553 +1
Branches 6707 6707
=======================================
+ Hits 17582 17583 +1
Misses 418 418
Partials 552 552 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review — safe to merge
✅ Approved — safe to merge Review summary Nits
Nits — 1 non-blocking
Review context
Contributor next steps
Signal definitions
Review detailsGenerated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative. The update adds a DIFF SCOPE rule to the system prompt and modifies composeAdvisoryNotes to wrap the Nits section in a collapsible <details> block, improving review focus. Corresponding unit tests are adjusted to check for the new markdown structure. The changes compile and the test expectations align with the new output format. Nits
🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
The false-close (soak finding)
An adversarial soak of 15 recent decisions found 14/15 correct but one confirmed false close (high severity): the bot auto-closed contributor PR metagraphed #1528 (green CI, mergeable, 5 new MCP tools + tests) on a hallucinated blocker — it claimed
loadArtifactDatawas "not imported → ReferenceError," but that function is defined locally (line 174) and used by ~24 existing call sites; CI passed, empirically disproving it.Root cause: the model sees only the diff, not the whole file, so new code referencing an existing out-of-diff symbol looks undefined. Both free models made the same mistake → consensus → close. (Full-file grounding is enabled but didn't prevent it here, so a prompt-level guard is the reliable fix; grounding effectiveness is a separate follow-up.)
Fix
<details>toggle (assessment + blockers stay visible), per request.Full suite green (3486). I'll reopen #1528 once this deploys.