Skip to content

Epic: PR improvement signal — quality-delta scoring as the positive-axis counterpart to slop-risk #4737

Description

@JSONbored

Context

Owner-directed research request (not an audit finding): could the review engine measure how much a PR
improves the codebase, not just whether it's risky? A research pass mapped every existing quality/value
signal in the stack before proposing anything, to make sure this is additive rather than a parallel
system. Findings, verified against current code:

  • src/signals/slop.ts answers "is this bad?" — deterministic, zero-LLM, weighted-sum risk score.
    By its own header comment it is "the ONLY thing allowed to gate (block)" — heuristic/AI judgments
    stay advisory. There is no positive-axis counterpart today.
  • The AI review call (src/services/ai-review.ts) reads only the unified diff (truncated at
    120,000 chars) and never the pre-change file content, even with the optional grounding flag on. Its
    output (ModelReview: assessment/blockers/nits/suggestions/confidence/inlineFindings) is entirely
    categorical — confidence is calibrated defect-certainty, not a quality/value axis.
  • "Readiness NN/100" (buildPublicReadinessScore, src/signals/engine.ts:4022) is a separate
    deterministic score built from 6 pure-metadata components (linked issue, duplicate-PR overlap, change
    size, validation-note presence, PR state, queue pressure). None of them inspect diff/code content.
  • qualityGateMode is the closest-sounding existing thing, and instructive: it is structurally
    prevented from ever blocking a PR, enforced at two independent layers
    (src/api/routes.ts downgradeQualityGateMode, src/signals/focus-manifest.ts:600 in
    resolveEffectiveSettings), with GateCheckPolicy.qualityGateMode's own doc comment
    (src/rules/advisory.ts:29-31) stating readiness "must never fail the Gate check." This is a
    deliberate architectural precedent this epic follows, not a constraint to work around
    : derived/
    heuristic judgments stay advisory; only the deterministic slop score gates anything.
  • REES's 55 analyzers (review-enrichment/src/analyzers/) include complexity.ts and
    coverage-delta.ts, both of which sound like before/after deltas but explicitly aren't — both
    disclaim this in their own header comments, because REES normally only receives diff hunks, not full
    files. Two other analyzers (doc-comment-drift.ts, exhaustiveness-drift.ts) already contain
    reconstructOldContent(newContent, patch) — a primitive that reverse-applies the diff patch onto the
    fetched post-change file to recover real pre-PR text. It has never been generalized past those two
    narrow structural-comparison use cases.
  • src/scoring/{model,preview}.ts is the actual Bittensor SN74 miner trust/reward incentive
    machinery (saturation curves, time decay, multipliers) — a different domain entirely, walled off by
    this repo's own "no trust scores / reward values" rule (CLAUDE.md). Not reusable infrastructure,
    and — more importantly — not a place this epic's signal should ever feed into.
    See Non-goals.
  • The public-comment sanitizer (four independently-implemented layers: src/signals/redaction.ts
    isPublicSafeText, src/queue-intelligence.ts sanitizePublicComment, a second same-named function
    in src/github/commands.ts, and src/signals/engine.ts containsPrivatePublicTerm) blanket-blocks the
    bare word "score" in any dynamically generated text. The existing "Readiness score: NN/100" line
    ships only because it's a static template string that bypasses the sanitizer entirely — any
    AI-authored sentence containing "score" gets silently dropped in full (toPublicSafe,
    ai-review.ts:520-528, catches and returns null — the whole note vanishes, not a redacted version of
    it). Every sub-issue that emits AI-authored prose must respect this.

The GPU/Ollama question, answered concretely

Self-host already has a fully-realized local-model path: AI_PROVIDER (src/env.d.ts:91-122) is an
ordered provider list (claude-code, codex, anthropic, ollama, ...) resolved once at boot into
AI_REVIEW_PLAN, with OLLAMA_AI_BASE_URL / OLLAMA_AI_MODEL / OLLAMA_AI_API_KEY already first-class
env vars, undefined on the hosted/cloud deployment. Because this epic's LLM-tier signal (see the
sub-issue below) is designed to extend the existing AI-review call's output schema rather than add a
new LLM call, it inherits whichever provider(s) the operator already has configured for review —
automatically, with zero new provider-selection surface.
A self-hoster running AI_PROVIDER=ollama
against local GPU hardware gets this signal on that same free/local path; a self-hoster on
AI_PROVIDER=anthropic pays only the marginal output-token cost of a couple of extra structured fields
on a call it already makes — not a second API call. There is nothing to build here beyond making sure
the new prompt/schema additions don't assume a cloud-only model's capabilities.

Design constraints (apply to every sub-issue below)

  1. Two tiers, not one blended percentage. A single "73% improvement" number is false precision
    neither a heuristic nor an LLM can honestly back. Follow this codebase's own convention
    (slop.ts's weighted sub-findings + band, buildPublicReadinessScore's named components): a
    deterministic tier of named, explainable structural sub-signals, plus a small-ordinal (not
    percentage) LLM-judged tier, kept separate and separately labeled.
  2. Advisory-only, never a gate input. Same rule qualityGateMode already enforces. No sub-issue in
    this epic wires the new signal into planAgentMaintenanceActions, auto-merge disposition, or any
    isConfiguredGateBlocker branch.
  3. Config-as-code, globally AND per-repo. Every sub-issue that adds a resolvable behavior follows
    this repo's standard parity checklist (DB migration + Drizzle schema, the settings resolver so
    .gittensory.yml > DB > defaults holds, OpenAPI, .gittensory.yml.example docs, resolution-precedence
    tests) and routes activation through the now-unified resolveConvergedFeature /
    resolveFeatureActivation core (src/review/feature-activation.ts, shipped in Unify per-feature *-wire.ts activation behind one shared resolver #4616) rather than a
    bespoke *-wire.ts — add a new ConvergedFeatureKey (e.g. improvementSignal) instead of
    reinventing precedence. Default OFF globally; an operator opts in, then individual repos can force it
    on/off relative to that default, same shape as every other converged feature.
  4. Sanitizer-safe by construction. Any AI-authored justification text must avoid "score" (and the
    rest of PUBLIC_UNSAFE_TERMS/FORBIDDEN_PUBLIC_COMMENT_WORDS) — prefer "improvement"/"value"/"gain".
    The actual computed band/label renders as static template copy (like "Readiness score: NN/100"
    already does), never as sanitizer-filtered dynamic text.
  5. This is a request for the full build, not a minimal slice — both tiers, real REES analyzer work
    (not just a schema bolt-on), and the two highest-value unlocks (maintainer triage view, MCP
    pre-submit tool), all in-scope for this epic.

Sub-issues

Phase A — foundation

Phase B — deterministic tier (REES)

Phase C — LLM tier

Phase D — surfacing

Phase E — unlocks

Non-goals / explicitly out of scope for this epic

  • Never feeds src/scoring/{model,preview}.ts or any Bittensor SN74 reward/trust mechanism.
    Architecturally walled off already, and this is exactly the kind of scalar a financially-motivated
    miner population will learn to game (Goodhart) the moment it touches real payout. If a future epic
    ever proposes crossing this line, that needs its own explicit, separately-scoped decision — not a
    quiet extension of this one.
  • Never a gate/auto-merge input. See design constraint 2 above.
  • No public per-contributor aggregate or leaderboard. Already forbidden
    (CONTRIBUTING.md: no public leaderboards, no raw ranking exposure).
  • Open-issue value prediction (predicting the value of solving an issue before any diff exists) is
    out of scope.
    Fundamentally harder (no diff to read), and a plausible-sounding v2 rather than part
    of this build.
  • No new LLM call / provider-selection surface. The LLM tier rides the existing AI-review call; see
    the GPU/Ollama section above.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.roadmapOn the Wave-2 agent-layer roadmap board (project 9)

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions