Skip to content

feat(review): surface a risk x value quadrant in the PR panel - #4964

Merged
JSONbored merged 1 commit into
mainfrom
feat/risk-value-quadrant-4745
Jul 11, 2026
Merged

feat(review): surface a risk x value quadrant in the PR panel#4964
JSONbored merged 1 commit into
mainfrom
feat/risk-value-quadrant-4745

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds a risk × value quadrant label (sub-issue H of epic Epic: PR improvement signal — quality-delta scoring as the positive-axis counterpart to slop-risk #4737, the epic's last core sub-issue) that
    crosses the existing slop-risk band (src/signals/slop.ts) with the deterministic structural-improvement
    band (Aggregate deterministic structural-improvement sub-score #4742) into a compact "risk: X · value: Y" string — exactly the maintainer 2x2 from this issue's
    own body (safe-but-worthless churn vs. risky-but-valuable work vs. actual slop vs. a fast-track candidate).
  • New exported pure function formatRiskValueQuadrant(slopBand, improvementBand) in src/signals/engine.ts.
    Degrades in two independent steps, never fabricating a reading for an axis a given pass didn't compute:
    absent improvementBand → risk-only label; absent slopBandundefined (nothing shown).
  • Threaded into the existing "Improvement" row (buildImprovementSignalRow) as a prefix on its Evidence
    cell, instead of a new row or a new fields: toggle key — the row already only renders when the
    improvementSignal converged feature resolves on for the repo, so reusing it means opted-out repos stay
    byte-identical for free, with no new key to hand-sync across .gittensory.yml.example /
    config/examples/gittensory.full.yml / gittensory-repo-focus-manifest.ts / .gittensory.yml.
  • src/queue/processors.ts: hoists a slopBand: SlopBand | null out of the try-block (mirroring the
    existing slopRisk hoist right next to it) so the quadrant can reuse the slop band already computed
    by the existing buildSlopAssessment call this pass — no second slop computation, no second
    resolveConvergedFeature/convergedFeatureActive resolution. Threaded into both panel builders
    (commentArgs for the legacy comment, and the buildPublicPrPanelSignalRows call for the unified-comment
    bridge) so the two never diverge, matching how every other row in this panel is already hand-mirrored.
  • Advisory/display-only: never touches evaluateGateCheck, isConfiguredGateBlocker, or any blocker path
    (epic design constraint 2). SlopBand/ImprovementBand are closed enums interpolated verbatim, so the
    quadrant clause is public-safe by construction — no sanitizer/containsPrivatePublicTerm check needed for
    the clause itself (verified with a dedicated regression test anyway, mirroring this epic's existing style).
  • Scope decision (issue's own "Optional" deliverable): the queue-level "high risk / low value" maintainer
    worklist is not built in this PR — spun out as an explicit fast-follow. I don't have a spawn_task
    tool in this environment, so per the issue's own fallback instruction I'm stating the decision here instead
    of filing a tracking issue myself: this repo's small-PR preference plus the issue's own "Optional" framing
    point at shipping the per-PR panel view first (this PR) and building the queue-level worklist as its own
    follow-up once the panel version proves useful, exactly as the issue body suggests.
  • No legacy/stale patterns found in the files this PR touches — both src/signals/engine.ts's
    improvement-row section and src/queue/processors.ts's slop-evidence block were shipped earlier today
    (feat(review): surface the improvement signal in the PR panel #4954/feat(review): surface the improvement signal in the PR panel #4954-adjacent) and are already clean; nothing adjacent needed modernizing in this pass.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck — clean.
  • npm run test:coverage (unsharded, full local run) — clean; diff-scoped cross-reference against
    coverage/lcov.info
    (not just the terminal summary): zero uncovered statements and zero uncovered
    branches on any changed/added line in src/signals/engine.ts or src/queue/processors.ts. Specifically
    confirmed both arms of every new conditional are hit: formatRiskValueQuadrant's slopBand === undefined
    branch (13/11 hits), its improvementBand === undefined branch (3/8 hits), buildImprovementSignalRow's
    quadrant ? … : … branch (4/11 hits), and both slopBand ?? undefined call sites in processors.ts
    (141/136 and 27/26 hits).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer
    boundaries — see below.
  • Additionally ran the full test:ci gate piece by piece (all green): db:migrations:check,
    db:schema-drift:check, selfhost:env-reference:check, selfhost:validate-observability,
    cf-typegen:check, test:engine-parity, test:driver-parity, the @jsonbored/gittensory-engine
    workspace test, build:miner, test:miner-pack, rees:test (1297 tests), ui:openapi:settings-parity,
    ui:version-audit, docs:drift-check, manifest:drift-check, command-reference:check, ui:test.
  • npx tsx scripts/check-engine-parity.ts directly — "22 hand-duplicated file pair(s) agree, no drift".
  • Full affected suite (signals-coverage.test.ts, ai-review-advisory.test.ts, queue.test.ts,
    queue-2/3/4/5.test.ts) — 735/735 passing. Broader npm run test:unit sanity sweep (the entire
    test/unit/ directory, not just touched files) — 679 files / 13573 tests, all passing.
  • New tests cover all four representative quadrant combinations (low-risk/low-value, low-risk/high-value,
    high-risk/low-value, high-risk/high-value) plus the "improvement signal unavailable" degraded case
    (risk-only label) plus the "slop band unavailable" case (undefined, nothing shown) — per this issue's
    own acceptance criteria.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private
    rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or
    optimization tactics — SlopBand/ImprovementBand values are closed enums, never free text, and a
    dedicated regression test asserts the quadrant clause can never carry forbidden vocabulary.
  • Auth/cookie/CORS/GitHub App/Cloudflare/session: N/A — no such surface touched.
  • API/OpenAPI/MCP: N/A — no route/schema/tool changed (confirmed via ui:openapi:check and
    ui:openapi:settings-parity, both clean/no-op for this diff).
  • UI changes: N/A — no apps/** files touched.
  • Public docs: N/A — no new fields: toggle key, so no .gittensory.yml.example update is needed (the
    quadrant reuses the existing improvementSignal key's row and gating end to end).

UI Evidence

This is a Markdown/text panel-comment change (an existing GitHub PR comment row), not a rendered browser UI
surface, so no screenshot/preview tooling in this environment applies (same precedent as the sibling #4954
PR that shipped the row this PR extends). Per this issue's own acceptance criterion for UI evidence, here is
the exact rendered text from this PR's own test assertions, in place of a screenshot:

  • Unit-level (buildPublicPrPanelSignalRows/buildPublicPrIntelligenceComment, both risk and value present):
    risk: low · value: minor — No structural-improvement signals were detected for this PR.
    
  • Unit-level, degraded (no slopBand threaded — byte-identical to before this PR):
    No structural-improvement signals were detected for this PR.
    
  • End-to-end (processJob → posted GitHub comment, unified-comment renderer, real buildSlopAssessment +
    buildStructuralImprovementAssessment output for the test fixture PR):
    | Improvement | ⚠️ ℹ️ None detected | risk: low · value: none — No structural-improvement signals were detected for this PR. |
    

Notes

Closes #4745
Part of epic #4737

Crosses the existing slop-risk band with the deterministic structural-improvement
band into a compact "risk: X · value: Y" label, prefixed onto the existing
Improvement row's Evidence cell -- the maintainer 2x2 from the epic's design doc
(safe-but-worthless churn vs. risky-but-valuable work vs. actual slop vs. a
fast-track candidate).

Reuses the already-computed slop band and structural-improvement assessment for
the current pass; no new fields: toggle key, no second convergedFeature
resolution, and no gate/blocker involvement. Repos that have not opted into the
improvementSignal converged feature see byte-identical panel output, since the
row (and therefore the quadrant prefix) only ever renders when that feature
resolves on for the repo.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.26%. Comparing base (4a662c8) to head (102f27c).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4964   +/-   ##
=======================================
  Coverage   94.26%   94.26%           
=======================================
  Files         447      447           
  Lines       38884    38893    +9     
  Branches    14167    14172    +5     
=======================================
+ Hits        36653    36662    +9     
  Misses       1574     1574           
  Partials      657      657           
Flag Coverage Δ
shard-1 46.54% <90.90%> (-0.28%) ⬇️
shard-2 33.58% <72.72%> (+0.27%) ⬆️
shard-3 31.56% <9.09%> (+0.14%) ⬆️
shard-4 32.51% <54.54%> (-0.51%) ⬇️
shard-5 34.50% <63.63%> (+0.96%) ⬆️
shard-6 45.51% <72.72%> (+0.27%) ⬆️

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

Files with missing lines Coverage Δ
src/queue/processors.ts 95.45% <100.00%> (+<0.01%) ⬆️
src/signals/engine.ts 97.48% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 04:16:14 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · unknown

⏸️ 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/**).

Review summary
This PR adds `formatRiskValueQuadrant(slopBand, improvementBand)` to src/signals/engine.ts and threads a pre-computed `slopBand` through both public-panel builders and the existing Improvement row, reusing the slop band already computed by `buildSlopAssessment` in processors.ts rather than recomputing it. The two-step degrade (no slopBand → undefined/no-op; no improvementBand → risk-only label) is implemented correctly and matches the row's existing bail-out behavior, and the row-reuse approach avoids adding a new toggle key across the four config surfaces. Tests cover all four quadrant combinations, both degrade paths, the byte-identical case when slopBand is omitted, an end-to-end processors.ts wiring test with a real slop computation, and a vocabulary-leak regression check — this is solid coverage for a small, well-scoped diff.

Nits — 6 non-blocking
  • The new doc comments on `slopBand` (engine.ts) and the hoisted `slopBand` in processors.ts are quite long relative to the one-line degrade rule they describe — could be trimmed without losing the 'never fabricate a reading' invariant.
  • `formatRiskValueQuadrant`'s em-dash-joined evidence string (`${quadrant} — ${evidence}`) is a new hardcoded separator; consider a small named constant if this pattern gets reused elsewhere in the panel.
  • Confirm issue Maintainer risk x value triage view #4745 is the correct eligible open issue this PR closes/links (the brief marks its coverage as 'partial') before treating this as fully issue-scoped.
  • Since `slopBand` is now a third optional axis threaded through two panel builders, a short comment near `buildPublicPrPanelSignalRows`'s signature noting the risk/value axis order (slopBand=risk, improvementBand=value) would help future readers avoid swapping the two positional args.
  • 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 1 reviewer
Linked issue ✅ Linked #4745
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: 48 registered-repo PR(s), 40 merged, 423 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 423 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
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, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 48 PR(s), 423 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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored
JSONbored merged commit b94589c into main Jul 11, 2026
18 checks passed
@JSONbored
JSONbored deleted the feat/risk-value-quadrant-4745 branch July 11, 2026 04:16
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.

Maintainer risk x value triage view

1 participant