Skip to content

fix(review): never render a ❌ for a non-Gittensor contributor (#5100) - #5774

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nickmopen:feat/neutral-contributor-signal
Jul 14, 2026
Merged

fix(review): never render a ❌ for a non-Gittensor contributor (#5100)#5774
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nickmopen:feat/neutral-contributor-signal

Conversation

@nickmopen

Copy link
Copy Markdown
Contributor

Summary

The readiness signal table had a row that visually contradicted its own evidence: for any author with no public Gittensor match, the Contributor context row rendered ❌ No public Gittensor match — a hard-failure ❌ (the same icon the table uses for real blockers) — right next to evidence that literally said "not a blocker." Every non-Gittensor contributor, i.e. the majority of PR authors on most repos, saw a red X on their own PR for something that was never a failure.

The root cause is that the unified signal table only had three states — ok (✅) / warn (⚠️) / fail (❌) — with no neutral state, so a non-blocking row had nowhere to land but a warning or a failure. This PR adds a proper neutral info (ℹ️) state and routes the contributor row through it.

This is issue #5100's explicitly-independent bug fix ("the ❌→✅/neutral fix as its own clearly-flagged bug fix ... should ship even if the rest of the table redesign takes longer"). The broader table redesign, row-cutting, and Points Forecast — which #5100 lists as open questions for the maintainer — are addressed as written recommendations at the bottom of this description, not implemented, since they are the maintainer's call.

The fix

  • src/signals/engine.tscontributorContextPanelResult: the non-confirmed branch now returns ℹ️ No public Gittensor match (was ❌ …). Evidence and action are unchanged; it was always "not a blocker."
  • src/review/unified-comment.ts — added info to the UnifiedSignalRow["state"] union and info: "ℹ️" to SIGNAL_ICON. A neutral/grey state that never implies ⚠️ or ❌.
  • src/review/unified-comment-bridge.tsrowState maps a leading ℹ️ to info (instead of falling through to warn), and rowResultText now strips a leading ℹ️ too.

Bonus: a latent double-icon bug this also fixes

Because ℹ️ previously fell through to warn and wasn't in the icon-strip set, the Improvement "none detected" row rendered as ⚠️ ℹ️ None detected in the unified comment — a warning icon plus an un-stripped legacy ℹ️, two icons for one informational row. With the neutral state it renders as a single, correct ℹ️ None detected.

Before → after (rendered signal table row)

Before:  | Contributor context | ❌ No public Gittensor match | octocat; not a blocker. |
After:   | Contributor context | ℹ️ No public Gittensor match | octocat; not a blocker. |

Before:  | Improvement | ⚠️ ℹ️ None detected | value: none |
After:   | Improvement | ℹ️ None detected     | value: none |

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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 — Closes #5100.

Validation

  • git diff --check
  • npm run typecheck (clean)
  • npm run test:coverage on the changed files — every changed line and both branches of the new ℹ️ mapping are exercised (bridge rowState/rowResultText, the SIGNAL_ICON literal, and the non-confirmed contributor branch); codecov/patch diff is at 100%.
  • Full vitest run unit + integration suite green.
  • npm run test:engine-parity + npm run test:live-gate-parity green (the golden-comment comparison jobs — confirms no fixture rendered the old icons).
  • npm run test:workers, build:mcp, test:mcp-pack, ui:openapi:check, ui:lint, ui:typecheck, ui:build — not exercised; this change is confined to src/review + src/signals comment rendering and touches no Worker binding, MCP surface, OpenAPI schema, or apps/loopover-ui/** code.

If any required check was skipped, explain why:

  • The MCP/UI/OpenAPI/workers checks are for surfaces this PR does not touch. The relevant gate — typecheck, the full unit/integration suite, the parity golden-comparison jobs, and codecov/patch on the diff — is green locally.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, PATs, private keys, raw trust scores, private rankings, or private maintainer evidence exposed. (The changed row surfaces only a public GitHub username and a "not a blocker" note; queue-4's not.toMatch(/wallet|hotkey|coldkey|reward|trust score/i) assertion still passes over the rendered comment.)
  • Public GitHub text stays sanitized, low-noise, and implies no compensation guarantees or optimization tactics.
  • Auth/cookie/CORS/GitHub App/Cloudflare/session — N/A, none changed.
  • API/OpenAPI/MCP — N/A, none changed.
  • UI changes — N/A. This renders a PR-comment Markdown table (backend), not an apps/loopover-ui page, so no UI Evidence screenshots apply. Before/after table rendering shown above.
  • Public docs/changelogs — N/A.

Notes

Per #5100's deliverables, the analysis it asks for (the parts that are maintainer decisions, not contributor changes):

Recommendation — which of the three low-value rows to cut

  • Cut "Validation posture" (validationEvidence). It restates the test/coverage signal that the dedicated coverage/test-evidence collapsible already presents in more detail — the same signal shown twice, exactly the redundancy Slim and redesign the main PR signals table (cut low-value rows, redesign Gittensor context, explore Points Forecast) #5100 flags.
  • Cut "Contributor workload" (openPrQueue key). It surfaces the author's concurrent open-PR count, which is already conveyed — where it is actually actionable (collision risk) — by the open-PR/related-work signal. As a standalone row it is low-actionability noise.
  • Keep "Change scope" (reviewLoad). It is the one row a maintainer uses to size review effort (size label / draft state / linked-issue burden) and is not redundant with anything else visible.

Net: three rows → one, raising signal density without losing anything a maintainer reads. Happy to do the cut in a follow-up once you confirm the call.

Recommendation — Points Forecast: keep as a prototype, do not ship yet

A pre-merge "projected points" row would combine the repo's emissionShare / labelMultipliers (from RepositoryRecord.registryConfig) with the PR's current type-label. The data sources exist, but the scoring algorithm it would forecast is Gittensor's, external to this repo — any formula we commit to here can silently drift from the real one and would read as a payout promise (a public-safe-wording violation). Recommendation: prototype only, gated behind a flag and worded strictly as a non-guaranteed estimate, until it can be validated against Gittensor's actual scoring. Not in this PR.

Note — surfacing per-repo credibility/score in the contributor row

#5100 also suggests replacing the confirmed-contributor evidence with the author's per-repo credibility/score from GittensorContributorSnapshot.repositories[]. That snapshot is not currently threaded into contributorContextPanelResult (its args are pr/profile/detection/confirmedMiner), so surfacing it is a small plumbing change best done as its own PR once the row redesign above is agreed. This PR ships the flagged ❌-contradiction fix, which stands on its own.

Closes #5100

…red#5100)

The Contributor context row showed "❌ No public Gittensor match" for any
author without a public Gittensor match — a hard-failure icon directly
next to evidence reading "not a blocker." The signal table had no neutral
state (only ok/warn/fail), so a non-blocking row had nowhere to land.

Add a neutral `info` (ℹ️) state to the unified table and route the
contributor row through it. This also fixes a latent double-icon bug: the
Improvement "none detected" row rendered as "⚠️ ℹ️" (a warn icon plus an
un-stripped legacy ℹ️) and is now a single, correct ℹ️.
@nickmopen
nickmopen requested a review from JSONbored as a code owner July 14, 2026 14:05
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (e4fd9c1) to head (ae9fb66).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5774   +/-   ##
=======================================
  Coverage   95.07%   95.07%           
=======================================
  Files         582      582           
  Lines       46216    46217    +1     
  Branches    14820    14821    +1     
=======================================
+ Hits        43940    43941    +1     
  Misses       1516     1516           
  Partials      760      760           
Flag Coverage Δ
shard-1 43.40% <100.00%> (-0.54%) ⬇️
shard-2 36.01% <66.66%> (+0.25%) ⬆️
shard-3 32.20% <66.66%> (-0.06%) ⬇️
shard-4 32.81% <100.00%> (-0.12%) ⬇️
shard-5 31.33% <66.66%> (-0.40%) ⬇️
shard-6 44.98% <100.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.57% <100.00%> (+<0.01%) ⬆️
src/review/unified-comment.ts 99.58% <100.00%> (ø)
src/signals/engine.ts 97.49% <ø> (ø)
🚀 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:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

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

5 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The diff adds a proper neutral `info`/ℹ️ state to the unified signal table and routes the non-Gittensor-contributor row and the 'none detected' improvement row through it instead of the misleading ❌/⚠️, fixing both the false-failure icon and a latent double-icon rendering bug. The regex and state-mapping changes are consistent across engine.ts, unified-comment.ts, and unified-comment-bridge.ts, and the accompanying tests assert the corrected `info` state and the fixed rendered string. This is a narrow, well-targeted bug fix that matches its stated scope (explicitly the independent bug-fix slice of #5100, deferring the broader table redesign as written recommendations).

Nits — 4 non-blocking
  • The PR only implements the explicitly-independent bug-fix slice of Slim and redesign the main PR signals table (cut low-value rows, redesign Gittensor context, explore Points Forecast) #5100 and defers the table redesign/row-cutting/Points Forecast as written recommendations rather than code — confirm the maintainer is fine merging this partial slice separately.
  • unified-comment-bridge.ts:120 mixes a literal `ℹ️` emoji in a character class regex — double check case where `ℹ️` variation selector (U+FE0F) presence/absence in incoming result strings still matches, since emoji regex matching across differing Unicode normalization can be brittle.
  • Consider a follow-up test asserting `SIGNAL_ICON.info` renders correctly end-to-end in the full unified comment table output, not just via the bridge unit test.
  • If other panel rows besides contributor-context and improvement could ever emit a neutral condition, audit them now while the `info` state is fresh in mind.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5100
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: 128 registered-repo PR(s), 78 merged, 9 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nickmopen; Gittensor profile; 128 PR(s), 9 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The change fixes a concretely-described, real-world misleading-icon bug (❌ shown for a non-blocking condition) with a clean, minimal, well-tested state addition rather than a speculative or oversized rewrite.
Linked issue satisfaction

Partially addressed
The PR ships the explicitly-independent ❌→info bug fix (new `info` state, updated icon mapping, and the contributor row now rendering ℹ️ instead of ❌) with solid test coverage, but it does not implement the row-cutting decision, the Contributor→Gittensor redesign to surface per-repo credibility/score with a profile link, or the Points Forecast prototype -- those are left as written recommendations

Review context
  • Author: nickmopen
  • 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: 128 PR(s), 9 issue(s).
  • Related work: Titles/paths share 3 meaningful terms. (issue #4801, issue #5100)
  • Related work: Titles/paths share 3 meaningful terms. (issue #4803, issue #5100)
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Check active issues and PRs before submitting.
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.
[BETA] Chat with Gittensory

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

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory 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/gittensory-commands

🟩 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@loopover-orb
loopover-orb Bot merged commit 9f03eb2 into JSONbored:main Jul 14, 2026
15 checks passed
This was referenced Jul 14, 2026
This was referenced Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slim and redesign the main PR signals table (cut low-value rows, redesign Gittensor context, explore Points Forecast)

1 participant