Skip to content

Extend ModelReview with an ordinal improvement/value judgment #4743

Description

@JSONbored

Context

Part of #4737 (parent epic — LLM tier). Independent of the REES sub-issues
(different codebase, src/services/ai-review.ts vs. review-enrichment/) — can be built in parallel
with Phase B.

The problem

The existing AI-review call (ModelReview type, src/services/ai-review.ts:388-406) is entirely
categorical: assessment (prose), blockers/nits/suggestions (string arrays), confidence
(calibrated defect-certainty — "how sure am I that my own blockers are real," not a quality/value
judgment), and inlineFindings. There is no axis anywhere in this schema for "is this change valuable /
well-targeted," and the model never receives pre-change file content by default (only the unified diff,
truncated at 120,000 chars — see buildUserPrompt, ai-review.ts:758-817), so any new judgment must be
inferable from the diff's own before/after hunk shape plus the PR description/linked issue, not a true
whole-file comparison (that's what the deterministic REES tier is for).

Fix

Add 1-2 new fields to ModelReview, populated by the same LLM call (no new call, no new cost beyond
marginal output tokens on an existing request) — e.g. valueAssessment: { magnitude: <ordinal band>; rationale: string }. Update the system prompt's required JSON shape (ai-review.ts:65) and add explicit
prompt guidance distinguishing this axis from confidence: the model should be asked "does this change
plausibly move the codebase forward, given the diff and stated intent" — not "am I sure this is bug-free"
(that's confidence) and not "is this risky" (that's slop.ts, which this call never touches).

Requirements

  • Ordinal, not a percentage. Same constraint as the deterministic tier — e.g. a 4-6 point band
    (unclear/minor/moderate/significant or similar), not a numeric score. A fake-precise "78% valuable"
    from an LLM is not defensible and undermines the whole signal's credibility.
  • rationale must be written assuming it will be sanitized before anything public sees it — avoid
    "score" and the rest of PUBLIC_UNSAFE_TERMS/FORBIDDEN_PUBLIC_COMMENT_WORDS in the prompt's own
    instructions to the model, not just hope the sanitizer catches it after the fact (recall: a sanitizer
    hit on AI-authored text drops the entire note, not just the offending phrase — see
    toPublicSafe/ai-review.ts:520-528). Add a test asserting the rendered rationale never trips
    isPublicSafeText/sanitizePublicComment for a representative set of prompts.
  • No new provider/model selection surface. This rides whatever AI_REVIEW_PLAN already resolved for
    the repo (including a self-hosted ollama entry pointed at local GPU hardware, per AI_PROVIDER's
    existing ordered-provider-list resolution in src/env.d.ts:91-122) — do not add a second
    provider-config path for this specific judgment.
  • Gated behind improvementSignal (the config-foundation sub-issue) — when the feature resolves off
    for a repo, the prompt/schema addition should not fire at all (save the tokens, don't just compute and
    discard).
  • Dual-review/consensus mode (AI_DUAL_REVIEW/AI_COMBINE) already exists for the categorical fields —
    decide and document how the new ordinal field combines across two reviewers when dual mode is on
    (e.g. take the more conservative of the two, or surface both) rather than leaving it undefined
    behavior.

Acceptance criteria

  • ModelReview carries a new ordinal value-assessment field, clearly distinct from confidence in
    both the type's doc comment and the system prompt's instructions to the model.
  • Feature-gated behind improvementSignal's resolved state — off by default, no extra prompt tokens
    spent when disabled.
  • No new LLM call, no new provider-selection config — confirmed by reading the diff, not just
    asserted.
  • Sanitizer-safety test: generate rationale text for representative inputs and assert none of it
    trips the public-comment sanitizers.
  • Documented behavior for dual-review/consensus mode.
  • Full branch coverage on the new prompt-building and response-parsing paths.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions