Skip to content

feat(review): surface the improvement signal in the PR panel - #4954

Merged
JSONbored merged 2 commits into
mainfrom
feat/improvement-signal-panel-section
Jul 11, 2026
Merged

feat(review): surface the improvement signal in the PR panel#4954
JSONbored merged 2 commits into
mainfrom
feat/improvement-signal-panel-section

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Combines the deterministic structural-improvement tier (Aggregate deterministic structural-improvement sub-score #4742) and, where also active, the LLM
    tier's composed value judgment (Extend ModelReview with an ordinal improvement/value judgment #4743) into a new "Improvement" panel row — gated behind the
    improvementSignal converged feature (Config-as-code foundation: improvementSignal as a converged feature (global + per-repo) #4738); nothing renders anything when it resolves false for
    the repo.
  • The band renders as static template copy (mirroring how "Readiness score: NN/100" already bypasses
    the public sanitizer safely), never as sanitizer-filtered dynamic text.
  • Both the deterministic findings and the LLM rationale are re-checked against
    containsPrivatePublicTerm at render time — defense in depth, even though both are already
    claimed safe upstream.
  • Wired into both buildPublicPrIntelligenceComment (legacy allRows) and
    buildPublicPrPanelSignalRows (unified-comment bridge) so the two never diverge, matching how the
    other seven rows are already hand-mirrored between them.
  • New fields.improvementSignal toggle in the existing per-row show/hide mechanism, documented in
    .gittensory.yml.example alongside its siblings.
  • Fixed a real pre-existing gap found while wiring the defense-in-depth check: containsPrivatePublicTerm's
    regex had "hotkey" but was missing its "coldkey" sibling.

Closes #4744
Part of epic #4737

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.
  • Linked issue: Closes PR panel: new advisory improvement-signal section #4744 (verified programmatically, see Notes).

Validation

  • npm run typecheck — clean.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • npx tsx scripts/check-engine-parity.ts — 22 hand-duplicated file pair(s) agree, no drift introduced.
  • Full affected test suite (signals-coverage.test.ts, ai-review-advisory.test.ts,
    queue.test.ts, queue-2/3/4/5.test.ts) — 727/727 passing.
  • Diff-scoped coverage cross-reference against coverage-final.json (not just the terminal
    summary): zero uncovered statements/branches on any added line in src/queue/processors.ts or
    src/signals/engine.ts. This included adding one new end-to-end test
    (queue-4.test.ts, "PR panel: new advisory improvement-signal section #4744: threads the improvement-signal row into the unified comment...")
    specifically to exercise the improvementSignalAllowed TRUE branch, since every other existing
    test in the suite leaves the feature off (the default) — that already covers the FALSE branch
    thousands of times over, but nothing previously drove the TRUE path at the processors.ts level.

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 — a dedicated regression test asserts the new row never leaks
    forbidden vocabulary regardless of what findings/rationale feed it, mirroring this repo's existing
    public-safety invariant-test style.
  • Auth/cookie/CORS/GitHub App/Cloudflare/session: N/A — no such surface touched.
  • API/OpenAPI/MCP: N/A — no route/schema/tool changed.
  • Public docs (.gittensory.yml.example) updated for the new fields.improvementSignal toggle.

UI Evidence

N/A — this is a Markdown/text panel-comment change, not a rendered UI surface; no screenshot tooling
applies. The exact rendered row text is shown verbatim in the new regression test's assertions and in
this PR's Validation notes above.

Notes

@JSONbored JSONbored self-assigned this Jul 11, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 11, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 0258ec5 Commit Preview URL

Branch Preview URL
Jul 11 2026, 02:58 AM

@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.25%. Comparing base (aa2bc7c) to head (0258ec5).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4954   +/-   ##
=======================================
  Coverage   94.24%   94.25%           
=======================================
  Files         445      445           
  Lines       38880    38900   +20     
  Branches    14160    14171   +11     
=======================================
+ Hits        36644    36664   +20     
  Misses       1577     1577           
  Partials      659      659           
Flag Coverage Δ
shard-1 46.79% <60.86%> (+0.04%) ⬆️
shard-2 33.26% <30.43%> (-0.04%) ⬇️
shard-3 31.51% <13.04%> (+0.10%) ⬆️
shard-4 33.03% <86.95%> (+0.03%) ⬆️
shard-5 33.52% <30.43%> (+0.03%) ⬆️
shard-6 45.22% <34.78%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
packages/gittensory-engine/src/focus-manifest.ts 99.17% <100.00%> (ø)
src/config/gittensory-repo-focus-manifest.ts 100.00% <ø> (ø)
src/queue/processors.ts 95.62% <100.00%> (+<0.01%) ⬆️
src/signals/engine.ts 97.47% <100.00%> (+0.02%) ⬆️
🚀 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 03:02:20 UTC

10 files · 1 AI reviewer · 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): .gittensory.yml (matched .gittensory.yml), src/queue/processors.ts (matched src/queue/**).

Review summary
This PR adds an advisory-only "Improvement" panel row combining the deterministic (#4742) and LLM (#4743) tiers, gated behind the improvementSignal converged feature, and threads it through both comment builders plus the field-toggle mechanism. The wiring (feature resolution, cache-hit/miss shape, defense-in-depth term filtering, focus-manifest field key) is consistent and well-tested, but the newly added queue-4.test.ts assertion contradicts the row's own stated invariant and is almost certainly the cause of the failing CI test shards.

Blockers

  • test/unit/queue-4.test.ts (the `expect(postedBody).toContain("| Improvement | ⚠️ ℹ️ None detected | ...")` assertion near the end of the new PR panel: new advisory improvement-signal section #4744 test) expects a leading "⚠️" before the band label, but `IMPROVEMENT_BAND_LABELS` in src/signals/engine.ts:~4650 defines `none: "ℹ️ None detected"` with no ⚠️ prefix, and the surrounding doc comment explicitly states "Advisory icons only (✅/ℹ️, never ⚠️/❌)" — either this test's expected string is simply wrong (likely why validate-tests (3)/(5) and validate are failing in CI) or the unified-comment renderer is silently prefixing a warning icon for this new row, which would visually contradict the PR's own advisory-only design intent; trace which one it is and fix the actual mismatch rather than adjusting the assertion to match whatever currently renders.
Nits — 6 non-blocking
  • src/signals/engine.ts: the extensive multi-paragraph doc comments on `buildImprovementSignalRow`/`improvementEvidenceText`/the exported args fields are quite long for this codebase's usual comment density — consider trimming to the non-obvious parts (the byte-identical/omission guarantee) and dropping the restatement of test assertions.
  • `improvementEvidenceText`'s cap-at-2-findings behavior ("(+N more.)") is only exercised via `signals-coverage.test.ts`'s synthetic 4-finding fixture; consider also asserting the boundary case of exactly 2 findings (no "+N more" suffix) for full branch coverage.
  • packages/gittensory-engine/src/focus-manifest.ts:438 and src/signals/engine.ts:4294/4684-4685 embed raw issue numbers (Aggregate deterministic structural-improvement sub-score #4742/Extend ModelReview with an ordinal improvement/value judgment #4743/PR panel: new advisory improvement-signal section #4744) inline in comments/arrays — harmless but the external tooling flags them as "magic numbers"; not worth changing.
  • Before merging, run the failing shards (validate-tests (3) and (5)) locally against this exact assertion in test/unit/queue-4.test.ts to confirm whether the fix belongs in the test string or in whatever renderer prepends the icon.
  • 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 #4744
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, 312 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 312 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: not available
  • Official Gittensor activity: 48 PR(s), 312 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
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

An AI reviewer flagged a likely defect, but its confidence was below this repository's configured close-confidence floor, so this is held for a maintainer to confirm instead of closing automatically. Resolve the flagged defect (see the review notes), or ask a maintainer to override.

Combines the deterministic structural-improvement tier (#4742) and the
LLM tier's composed value judgment (#4743) into a new advisory panel
row, gated behind the improvementSignal converged feature (#4738).
Re-checks both inputs against the public-safety filter as defense in
depth, and fixes containsPrivatePublicTerm's regex (it had "hotkey"
but was missing its "coldkey" sibling).

Closes #4744
Part of epic #4737
… config copies

.gittensory.yml.example and src/config/gittensory-repo-focus-manifest.ts's
bundled copy already documented the new fields.improvementSignal toggle;
.gittensory.yml and config/examples/gittensory.full.yml are two more
copies of the same block that config-templates.test.ts and
gittensory-focus-manifest.test.ts require to stay aligned.
@JSONbored
JSONbored force-pushed the feat/improvement-signal-panel-section branch from b3c2c29 to 0258ec5 Compare July 11, 2026 02:55
@JSONbored
JSONbored merged commit c178616 into main Jul 11, 2026
20 checks passed
@JSONbored
JSONbored deleted the feat/improvement-signal-panel-section branch July 11, 2026 03:10
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.

PR panel: new advisory improvement-signal section

1 participant