Skip to content

review: collapse signal table into decision drivers + advisory fold - #6083

Merged
JSONbored merged 3 commits into
mainfrom
review/comment-decision-driver-chips
Jul 15, 2026
Merged

review: collapse signal table into decision drivers + advisory fold#6083
JSONbored merged 3 commits into
mainfrom
review/comment-decision-driver-chips

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #6067.

Stacked on #6077 (same function, sequential edit — this PR's diff is intentionally scoped to just the table change; GitHub will show only the incremental diff against that branch, and auto-retarget to main once #6077 merges).

Summary

  • UnifiedSignalRow gets a new optional gates?: boolean field. panelRowsToSignalRows (the bridge that converts the legacy 4-col panel rows into the unified renderer's rows) sets gates: row.key === "gateResult" — the ONLY row among the panel's 7 that can actually move the verdict; every other row's own Evidence/Action text already says it's advisory (e.g. "No action.", "Advisory only.").
  • The always-rendered table is replaced with:
    • Decision drivers — an always-visible bullet list: the synthetic "Code review" row (unchanged, still never subject to review.fields) plus any gates: true row.
    • Context & advisory signals — a new collapsed <details> containing the same Signal/Result/Evidence table markup as before, scoped to the non-gating rows. Omitted entirely when there's nothing advisory to show.
  • review.fields.<key> visibility toggles are unaffected — filtering still happens on the raw panel rows (by .key) before this split, so a hidden row stays hidden in either bucket.
  • Neither section is gated by review.comment_verbosity: quiet, matching the old table's own invariant (it was never dropped by quiet either) — only decorative detail (Nits, extraCollapsibles) is.

Test plan

  • npx vitest run test/unit/unified-comment.test.ts test/unit/unified-comment-bridge.test.ts test/unit/signals-coverage.test.ts — 224/224 pass (updated existing table-format assertions to the new bullet format + 3 new tests covering the gates:true split and its edge cases)
  • npx tsc --noEmit clean
  • npm run docs:drift-check, npm run manifest:drift-check, npm run command-reference:check — pass
  • Grepped for other consumers of the removed signalTable/old row shape — none found
  • Coverage-checked the exact diff hunks — no uncovered statements/branches within changed lines

@superagent-security

Copy link
Copy Markdown
Contributor

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

Base automatically changed from review/comment-verdict-visual-cleanup to main July 15, 2026 08:40
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (0760e1f) to head (c1f0695).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6083   +/-   ##
=======================================
  Coverage   95.24%   95.24%           
=======================================
  Files         595      595           
  Lines       47035    47062   +27     
  Branches    15015    15021    +6     
=======================================
+ Hits        44797    44824   +27     
  Misses       1493     1493           
  Partials      745      745           
Flag Coverage Δ
shard-1 43.97% <72.00%> (-0.04%) ⬇️
shard-2 36.61% <52.00%> (-0.01%) ⬇️
shard-3 31.98% <68.00%> (-0.08%) ⬇️
shard-4 33.82% <100.00%> (+0.69%) ⬆️
shard-5 31.46% <44.00%> (-0.76%) ⬇️
shard-6 44.84% <52.00%> (+0.34%) ⬆️

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

Files with missing lines Coverage Δ
src/review/unified-comment-bridge.ts 99.58% <100.00%> (+<0.01%) ⬆️
src/review/unified-comment.ts 99.61% <100.00%> (+0.02%) ⬆️

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

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 LoopOver review result - fixes required

Review updated: 2026-07-15 09:09:14 UTC

4 files · 1 AI reviewer · no blockers · readiness 80/100 · CI failing · dirty

🛑 Suggested Action - Manual Review

Review summary
This diff splits the previously single always-rendered signal table into an always-visible 'Decision drivers' bullet list (Code review + any `gates: true` row) and a collapsed 'Context & advisory signals' table for everything else, driven by a new optional `gates` field set only for the panel's 'Gate result' row in `panelRowsToSignalRows`. The logic is correctly gated: `review.fields` filtering still happens upstream on raw panel rows so per-row visibility is unaffected, and `comment_verbosity: quiet` correctly leaves both new sections untouched (matching the old table's invariant). Tests cover both branches of the bare-row rendering (present/absent result and evidence) and the gates:true/false split, and the diff is well-scoped to the stated #6067 issue.

Nits — 6 non-blocking
  • The new JSDoc blocks above `codeReviewRow`, `decisionDriverBlock`, and `advisorySignalsTable` in src/review/unified-comment.ts are multi-paragraph comment blocks explaining rationale that mostly belongs in the PR description, not inline — consider trimming to one-line explanations of non-obvious invariants only.
  • `signalRowLine` (unified-comment.ts) and the inline row-mapper inside `advisorySignalsTable` both re-implement result/evidence formatting slightly differently (bullet vs table cell) with no shared helper — minor duplication, not urgent to unify.
  • `panelRowsToSignalRows` in unified-comment-bridge.ts now always sets `gates: row.key === "gateResult"` — worth double-checking that `"gateResult"` is in fact the stable/only key used for the loopover gate row today, since a rename there would silently move it into the advisory fold with no compile-time signal.
  • Consider bolding or otherwise visually distinguishing the synthetic 'Code review' row within the new Decision drivers bullet list, since the old table's `i === 0` bold treatment for that row was dropped without an equivalent replacement.
  • If more `gates: true` rows are added in the future, consider asserting/documenting an expected ordering (e.g. Code review always first) since `decisionDriverBlock` currently just concatenates arrays in insertion order.
  • 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.

CI checks failing

  • validate
  • codecov/patch — 74.07% of diff hit (target 99.00%)
  • validate-tests (1)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #6067
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
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: 43 registered-repo PR(s), 35 merged, 360 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 43 PR(s), 360 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor
Review context
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 steps 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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://gittensory.aethereal.dev/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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 15, 2026
The always-rendered 9-row Signal/Result/Evidence table only ever had
two decision-authoritative rows (Code review, Gate result); the other
seven already say "No action." / "Advisory only." in their own
Evidence/Action text. Split it into an always-visible "Decision
drivers" bullet list (only rows marked gates: true, plus the synthetic
Code review row) and a collapsed "Context & advisory signals" fold for
everything else -- same underlying data, same review.fields
visibility, no longer competing for attention with the blockers.

Closes #6067
#6067 replaced the always-rendered table's bold "**Code review**"
first row with the "Decision drivers" bullet list -- this end-to-end
integration test (exercising the real renderer through processGitHubWebhook,
not a scoped unit test) still asserted the old bold-table-row text.
The structured fix-context data (path/line/instruction/suggested diff)
already existed via buildFixHandoffBlock, but only ever rendered into
its own disconnected, all-severities-combined "Fix handoff" collapsible
-- never attached to the blockers a reader is actually looking at.

Split fixHandoffBlocks by severity: blocker-severity findings now
render as their own "Copy AI fix context" collapsible directly under
each blocker (still gated by the existing review.fixHandoff toggle);
nit-severity findings keep going into the combined "Fix handoff"
collapsible as before. The whole-PR "Copy for AI agents" prompt is
unchanged and stays available as the aggregate option.

Closes #6068
@JSONbored
JSONbored force-pushed the review/comment-decision-driver-chips branch from dca092b to c1f0695 Compare July 15, 2026 09:26
@JSONbored
JSONbored merged commit d7b0810 into main Jul 15, 2026
17 checks passed
@JSONbored
JSONbored deleted the review/comment-decision-driver-chips branch July 15, 2026 09:36
andriypolanski pushed a commit to andriypolanski/gittensory that referenced this pull request Jul 15, 2026
…follow-up)

The unified-comment bridge became the sole PR-comment renderer in JSONbored#6128,
which left the unifiedComment ConvergedFeatureKey and its
LOOPOVER_REVIEW_UNIFIED_COMMENT env flag declared but functionally
inert. Since the drift-check gate requires every declared
LOOPOVER_REVIEW_* flag to be documented, and there was no working
precedent for a retired-but-still-required flag, this fully removes the
key/flag from the resolver, config-as-code examples, self-hosting docs,
and env declarations (regenerating worker-configuration.d.ts via
cf-typegen) rather than leaving stale references. Also fixes the
"signal table" description in docs.how-reviews-work.tsx, left over from
the earlier decision-drivers redesign (JSONbored#6083), and a timing-flaky
GraphQL single-flight test unrelated to this change but caught by the
same full-suite run (crypto.subtle.digest is backed by a real OS thread
pool, so two concurrent callers with identical inputs can complete in
either order under load; stubbed with a deterministic pure-microtask
digest for the one test that exercises the race).
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review: collapse the 9-row signal table into decision-driver chips + one advisory fold

1 participant