Skip to content

feat(review): upgrade inlineComments + fixHandoff to full config-as-code substitutes - #4116

Merged
JSONbored merged 2 commits into
mainfrom
feat/inline-comments-fix-handoff-4099
Jul 8, 2026
Merged

feat(review): upgrade inlineComments + fixHandoff to full config-as-code substitutes#4116
JSONbored merged 2 commits into
mainfrom
feat/inline-comments-fix-handoff-4099

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • shouldRequestInlineFindings (src/review/inline-comments.ts) and shouldEmitFixHandoff (src/review/fix-handoff.ts) previously required manifestToggle === true && envFlag && isConvergenceRepoAllowed(...) — ALL THREE — so a repo could restrict via config but never fully enable either feature without also being on the GITTENSORY_REVIEW_REPOS cutover allowlist.
  • Both now: (1) the operator's env flag remains an absolute master kill-switch (off ⇒ always false, never bypassable by any per-repo config — consistent with every other converged feature via resolveConvergedFeature); (2) an explicit review.inlineComments/review.fixHandoff: true|false in .gittensory.yml now FULLY controls the feature by itself, bypassing the cutover allowlist entirely; (3) unset stays byte-identical to today (the allowlist alone was never sufficient on its own for either feature — a real, deliberate departure from rag/reputation/safety/unifiedComment/grounding, which already fall back to the allowlist when unset).
  • Part of Epic: config-as-code should fully control every review feature, not just restrict it #4092 (config-as-code epic). Closes Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes #4099.

Update after gittensory-orb review feedback: the bot correctly flagged two things, both now fixed:

  1. The initial version widened resolveReviewPromptOverrides's inlineComments field to a boolean | null tri-state, but neither gate function ever consumed the distinction (both only check === true) — reverted to a plain collapsed boolean, matching every sibling field on that object. Simpler, same behavior.
  2. shouldEmitFixHandoff's type had drifted to boolean | undefined while shouldRequestInlineFindings had gained boolean | null | undefined — both now have the identical boolean | undefined shape, true parity.
  3. The linked-issue-satisfaction check flagged that this PR makes the env flag an absolute, never-bypassable kill-switch, which reads as a narrower scope than issue Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes #4099's original text ("even if the env var/allowlist would otherwise say no") implied. The implementation is correct — it's consistent with resolveConvergedFeature's established design for every other converged feature, and an operator-level kill-switch should not be overridable by lower-trust, repo-level config. The issue text was imprecise on this point; corrected there directly.

Scope

Validation

  • git diff --check
  • npm run typecheck — clean.
  • Targeted tests: test/unit/focus-manifest.test.ts, test/unit/config-templates.test.ts, test/unit/inline-comments.test.ts, test/unit/review-fix-handoff.test.ts, test/unit/fix-handoff-collapsible.test.ts, test/unit/fix-handoff-render.test.ts, test/unit/signals-coverage.test.ts, test/unit/visual-config-wiring.test.ts — all green (661 tests).
  • Full test/unit/queue.test.ts integration suite — green (697 tests).
  • Scoped coverage check on both changed gate files (src/review/inline-comments.ts, src/review/fix-handoff.ts) — 100% line and branch coverage confirmed via lcov BRDA records; src/signals/focus-manifest.ts diff is comment-only after the simplification.
  • npm audit --audit-level=moderate — 0 vulnerabilities.

Safety

  • No secrets/wallets/hotkeys/trust-scores exposed.
  • N/A — no auth/session/CORS changes.
  • N/A — no API/OpenAPI/MCP surface touched.
  • N/A — no UI changes.

@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 01:43:54 UTC

7 files · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/queue/processors.ts (matched src/queue/**).
Nits — 2 non-blocking
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers No AI review summary
Linked issue ✅ Linked #4099
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low 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: 51 registered-repo PR(s), 43 merged, 509 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 509 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The diff migrates both shouldRequestInlineFindings and shouldEmitFixHandoff exactly per the resolved design: the env flag is now an absolute kill-switch, an explicit manifest true/false fully controls the feature bypassing the cutover allowlist, and unset preserves the prior always-off behavior (verified by explicit regression tests). It also touches resolveReviewPromptOverrides and adds tests ana

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 509 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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.

🟩 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.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (92dfae1) to head (7218186).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4116   +/-   ##
=======================================
  Coverage   93.64%   93.65%           
=======================================
  Files         384      384           
  Lines       35856    35860    +4     
  Branches    13159    13159           
=======================================
+ Hits        33579    33583    +4     
  Misses       1618     1618           
  Partials      659      659           
Files with missing lines Coverage Δ
src/queue/processors.ts 95.15% <ø> (ø)
src/review/fix-handoff.ts 100.00% <100.00%> (ø)
src/review/inline-comments.ts 100.00% <100.00%> (ø)
src/signals/focus-manifest.ts 99.62% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added 2 commits July 7, 2026 18:31
…ode substitutes

An explicit review.inlineComments/fixHandoff: true|false in .gittensory.yml
now fully controls each feature, bypassing the GITTENSORY_REVIEW_REPOS
cutover allowlist entirely. Unset stays byte-identical to before (the
allowlist alone was never sufficient on its own for either feature).
The operator's env flag remains an absolute master kill-switch either way.

Closes #4099.
…switch docs

- Revert the unnecessary boolean|null tri-state on shouldRequestInlineFindings
  and resolveReviewPromptOverrides's inlineComments field -- the function only
  ever checks '=== true', so null and undefined were functionally identical;
  collapsing back to a strict boolean (matching every sibling field) removes
  complexity that served no purpose, per gittensory-orb review feedback.
- Make shouldEmitFixHandoff match shouldRequestInlineFindings exactly (both
  boolean | undefined, same shape) instead of drifting to boolean | null.
- Clarify in both doc comments that the operator's env flag is an ABSOLUTE
  kill-switch never bypassable by per-repo config, consistent with every
  other converged feature (resolveConvergedFeature) -- addresses a linked-
  issue-satisfaction flag about apparent scope drift from the issue text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes

1 participant