Skip to content

feat(signals): add deterministic structural-improvement assessment - #4822

Merged
JSONbored merged 1 commit into
mainfrom
feat/improvement-signal-deterministic
Jul 11, 2026
Merged

feat(signals): add deterministic structural-improvement assessment#4822
JSONbored merged 1 commit into
mainfrom
feat/improvement-signal-deterministic

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds src/signals/improvement.ts (sub-issue E of epic Epic: PR improvement signal — quality-delta scoring as the positive-axis counterpart to slop-risk #4737): a buildStructuralImprovementAssessment entrypoint that mirrors slop.ts's shape (input type, weighted-findings formula, banding function) but on the positive axis. It combines four deterministic inputs into { improvementScore, band, findings }:
    • REES's complexity-delta findings (Real complexity-delta analyzer (true before/after comparison) #4740) -- fires when at least one function's complexity genuinely dropped (mixed regressions/improvements in the same array are filtered by sign, not just counted).
    • REES's duplication-delta findings (Real duplication-delta analyzer (before/after comparison) #4741) -- every entry is already a resolved pair by construction, so presence alone is the positive signal.
    • A patchCoverageDeltaPercent figure, intended to reuse Codecov's own codecov/patch before/after number rather than recomputing it.
    • A test-evidence signal that reuses slop.ts's own buildMissingTestEvidenceFinding rather than re-deriving isCodeFile/isTestFile heuristics.
  • Deterministic tier only -- no LLM call. The LLM-tier judgment (ModelReview.valueAssessment, already shipped) is a separate axis combined at the surfacing layer in a later sub-issue (PR panel: new advisory improvement-signal section #4744), never blended into this score.
  • Explicit insufficient-signal band for a PR where none of the four inputs had anything to measure (e.g. docs-only), distinct from a genuine none verdict (measured, found no improvement) -- both share improvementScore === 0, so the band is the only way to tell them apart.
  • This module carries no gate/blocker power and is not wired into anything yet -- it is a pure, standalone computation consumed only by its own tests, the same activation-wiring-only pattern #4738/#4753 already used for this epic's feature-flag foundation. The surfacing sub-issue (PR panel: new advisory improvement-signal section #4744) wires a real caller later.

Two of the four inputs are honest gaps, not live data, and this PR does not attempt to close them (out of scope for this sub-issue):

  • REES (review-enrichment/) is a separate deployable -- its own package.json/tsconfig.json, not a root workspace member, not in the root tsconfig.json include -- so ComplexityDeltaFinding/DuplicationDeltaFinding are not importable here; the input type structurally mirrors their shape instead (ComplexityDeltaLike/DuplicationDeltaLike).
  • I traced src/review/enrichment-wire.ts end-to-end: today it only splices REES's pre-rendered { promptSection, systemSuffix } text into the AI review prompt and never parses brief.findings at all -- there is currently no channel that threads REES's structured findings into the main app. Likewise, src/review/grounding-wire.ts/src/review/unified-comment.ts only carry Codecov's check-run text summary (e.g. "60% of diff hit (target 97%)") for human display; nothing in this codebase's signal pipeline extracts a structured before/after number from Codecov today. Wiring a live source for both is follow-up work for a later sub-issue, not this one.

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 (2 files: the new module + its test file).
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves -- Closes Aggregate deterministic structural-improvement sub-score #4742.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally -- src/signals/improvement.ts is 100% statements/branches/functions/lines in isolation; full unsharded suite (704 files) passed with 0 failures.
  • 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 behavior has unit tests for every branch, including both sides of every ??/ternary/&& (mixed-sign complexity arrays, NaN/Infinity coverage figures, empty-path changed files, undefined vs explicit-empty arrays) plus golden-fixture band-boundary and determinism/invariant tests mirroring slop.test.ts's own style.

Full local gate also run beyond the checklist above: npm run 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 suite, build:miner, test:miner-pack, rees:test, ui:openapi:settings-parity, ui:version-audit, docs:drift-check, manifest:drift-check, engine-parity:drift-check, command-reference:check, and ui:test -- all green. engine-parity:drift-check confirms this new file has no mirrored twin under packages/gittensory-engine/src/signals/ (verified, not assumed), so it is exempt from the hand-duplicated-pairs check.

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 and low-noise -- every finding's detail/action/publicText is either static prose or interpolates only integer/numeric counts (mirroring slop.ts's own "only an integer count is interpolated" pattern), and tests assert the forbidden-term regex never matches.
  • N/A -- no auth, cookie, CORS, GitHub App, Cloudflare, or session changes.
  • N/A -- no API/OpenAPI/MCP surface changes.
  • N/A -- no UI changes.
  • N/A -- no visible UI/frontend/docs change; this module is not wired into any UI or panel yet.
  • N/A -- no changelog edit (not a release-prep PR).

UI Evidence

Not applicable -- this PR has no visible UI, frontend, or docs change. src/signals/improvement.ts is a pure computation module consumed only by its own unit tests in this PR.

Notes

…4742)

Adds src/signals/improvement.ts, the positive-axis counterpart to
slop.ts's risk-only score (sub-issue E of epic #4737): a
buildStructuralImprovementAssessment entrypoint that combines four
deterministic inputs -- REES's complexity-delta and duplication-delta
findings (#4740/#4741), a patch-coverage-delta figure reusing
Codecov's own codecov/patch numbers, and a test-evidence signal
reusing slop.ts's own buildMissingTestEvidenceFinding -- into one
{ improvementScore, band, findings } result, mirroring
SlopAssessment's shape deliberately.

Deterministic tier only, matching the epic's tier split: no LLM call
lives here, and the LLM-tier judgment (ModelReview.valueAssessment,
already shipped) is combined at the surfacing layer in a later
sub-issue, never blended into this score. This module carries no
gate/blocker power -- it is a pure, standalone computation consumed
only by its own tests, the same activation-wiring-only pattern the
epic's foundation sub-issue already used.

Bands are none/minor/moderate/significant, plus a distinct
insufficient-signal band for a PR where none of the four inputs had
anything to measure (e.g. docs-only) -- so an analyzer's silence is
never misread as "measured, found no improvement" (score 0 alone
cannot distinguish those two cases).

Two of the four inputs are honest gaps rather than live data: REES is
a separate deployable (not a workspace member, not in the root
tsconfig), so the complexity/duplication-delta types are structurally
mirrored here rather than imported, and nothing in this codebase
currently threads REES's structured findings (as opposed to its
rendered prompt text) into the main app, or extracts a structured
number from Codecov's check-run text. Wiring a live source for both is
follow-up work, not part of this change.
@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.24%. Comparing base (20c412b) to head (6cc59dc).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4822   +/-   ##
=======================================
  Coverage   94.23%   94.24%           
=======================================
  Files         442      443    +1     
  Lines       38845    38892   +47     
  Branches    14150    14173   +23     
=======================================
+ Hits        36607    36654   +47     
  Misses       1577     1577           
  Partials      661      661           
Flag Coverage Δ
shard-1 51.28% <0.00%> (-0.21%) ⬇️
shard-2 45.45% <100.00%> (+0.04%) ⬆️
shard-3 41.82% <0.00%> (-0.14%) ⬇️
shard-4 52.10% <0.00%> (+0.09%) ⬆️

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

Files with missing lines Coverage Δ
src/signals/improvement.ts 100.00% <100.00%> (ø)
🚀 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 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-11 01:34:51 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This adds src/signals/improvement.ts, a pure, deterministic PR-improvement scorer that mirrors slop.ts's shape (weighted findings, band function) on the positive axis, combining REES complexity/duplication deltas, a patch-coverage delta figure, and reused test-evidence detection into a 0-100 score and band. It is explicitly not wired into any caller yet (matches the prior #4738/#4753 activation-only pattern for this epic) and carries no gate power, so there is no reachable production path to break. The logic is internally consistent — weights sum to exactly 100, the insufficient-signal vs none distinction correctly follows from hasApplicableSignal, the mixed-sign complexity filtering and NaN/Infinity guarding are correct — and the 443-line test suite exercises golden fixtures across every band boundary plus each builder function in isolation, all against real (if currently uncalled) code paths rather than fabricated states.

Nits — 6 non-blocking
  • src/signals/improvement.ts imports buildMissingTestEvidenceFinding and SlopChangedFile from ./slop and isCodeFile from ./path-matchers — worth double-checking in review that slop.ts's changedFiles/tests/testFiles parameter shapes haven't drifted from what's assumed here, since that file wasn't in this diff.
  • The four weights (35/35/20/10) and band thresholds (31/60) are well-commented but still magic numbers scattered across improvement.ts and improvementBandFor; consider deriving the thresholds from IMPROVEMENT_WEIGHTS (e.g. a documented MODERATE_MIN = min(weights) constant) so a future weight change can't silently desync the band boundaries from the rubric text.
  • IMPROVEMENT_RUBRIC_MARKDOWN duplicates the band ranges as a hand-written string; a test asserting substring containment (as done here) won't catch the string drifting out of sync with improvementBandFor's actual thresholds if either is edited alone.
  • Since this module is confirmed unwired (per the PR description and the absence of any caller in this diff), it'd strengthen the epic tracking to add a one-line TODO/reference to PR panel: new advisory improvement-signal section #4744 near buildStructuralImprovementAssessment's export, consistent with the header comment, so a future contributor scanning exports finds the wiring plan quickly.
  • Consider a lightweight type-level test (or a comment) confirming ComplexityDeltaLike/DuplicationDeltaLike are kept in sync with REES's real types in review-enrichment/src/types.ts, since they're intentionally not imported and could silently drift.
  • 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.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4742
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, 275 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 275 issue(s).
Gate result ✅ Passing No configured blocker found.
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), 275 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 65422c8 into main Jul 11, 2026
16 checks passed
@JSONbored
JSONbored deleted the feat/improvement-signal-deterministic branch July 11, 2026 01:41
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. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

Aggregate deterministic structural-improvement sub-score

1 participant