Skip to content

fix(review): resolve dead aiReviewCloseConfidence floor with a configurable disposition - #4656

Merged
JSONbored merged 2 commits into
mainfrom
fix/ai-review-low-confidence-disposition-4603
Jul 10, 2026
Merged

fix(review): resolve dead aiReviewCloseConfidence floor with a configurable disposition#4656
JSONbored merged 2 commits into
mainfrom
fix/ai-review-low-confidence-disposition-4603

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Resolves #4603: aiReviewCloseConfidence has been a dead floor since commit 311b7613d (#1781) —
isConfiguredGateBlocker voided the value and a sub-floor ai_consensus_defect/ai_review_split
finding always one-shot-closed a contributor PR under aiReviewMode: block, regardless of the AI
reviewers' own calibrated confidence. Five other comments across the codebase still described the old
(correct) "downgrades to a hold" behavior, which the #1781 change silently broke without updating them.

Per the maintainer decision recorded on the issue, this ships a new aiReviewLowConfidenceDisposition
field ("one_shot" | "hold_for_review" | "advisory_only", default hold_for_review) that governs what
happens to a sub-floor finding:

  • hold_for_review (shipped default) — the defect still blocks the gate (merge stays blocked), but
    routes the would-be one-shot close through the same held-for-manual-review mechanism the disposition
    planner already uses for migrationCollisionHold/unlinkedIssueMatchHold, instead of inventing a new one.
  • one_shot — today's live (pre-fix) behavior, unchanged. Opt-in only.
  • advisory_only — a sub-floor finding drops to fully non-blocking.

At-or-above-floor confidence behaves identically across all three values — the floor only matters below
threshold.

What changed

  • Gate logic (src/rules/advisory.ts): isConfiguredGateBlocker's ai_consensus_defect/ai_review_split
    branch now reads the resolved disposition; new exported resolveAiReviewLowConfidenceHold resolves the
    { reason, comment } hold object for a gate failure driven solely by a sub-floor AI-judgment finding.
  • Disposition planner (src/settings/agent-actions.ts): new aiReviewLowConfidenceHold input narrows
    willClose's conclusion === "failure" arm only — a genuinely different adverse signal (red CI, a base
    conflict, or a concrete blocker alongside the AI defect) still closes normally. The generic manual-review
    label fallback now carries the hold's comment.
  • Gate-decision engine twin (packages/gittensory-engine/src/advisory/gate-advisory.ts): mirrored so the
    MCP predict_gate tool's gate-decision core stays in sync (checkGateDecisionVersionBump requires either a
    matching edit to both twins or an engine version bump — this PR edits both).
  • Full new-field wiring: migration 0140_ai_review_low_confidence_disposition.sql (DB column, default
    hold_for_review), src/db/schema.ts/src/db/repositories.ts (resolver + parser + upsert round-trip),
    src/openapi/schemas.ts + regenerated apps/gittensory-ui/public/openapi.json, .gittensory.yml
    (gate.aiReview.lowConfidenceDisposition, config-as-code override) via
    packages/gittensory-engine/src/focus-manifest.ts + src/signals/focus-manifest.ts, and the maintainer
    /ai-review self-serve route + the internal full-replace settings route in src/api/routes.ts.
  • Stale-comment cleanup — all 6 locations the original audit found now describe the actual (fixed)
    behavior: src/types.ts (AdvisoryFinding.confidence, RepositorySettings.aiReviewCloseConfidence),
    src/services/ai-review.ts (×3: ModelReview.confidence, consensusDefectOf, runGittensoryAiReview),
    src/queue/processors.ts (×2: the consensus-defect and split finding construction), and
    isConfiguredGateBlocker itself.
  • .gittensory.yml.example + config/examples/gittensory.full.yml document the new
    gate.aiReview.lowConfidenceDisposition field alongside the existing closeConfidence.

Deviations from the issue's suggested shape

  • The issue's acceptance criteria listed the wiring template as "migration + resolver + OpenAPI +
    .gittensory.yml.example". I additionally wired the maintainer self-serve /ai-review route and the
    internal full-replace /v1/internal/repos/:owner/:repo/settings route (mirroring the sibling
    aiReviewMode field's own dashboard-settable pattern), since the field is explicitly DB-backed/
    dashboard-settable per the decision comment — leaving it with no write path other than raw SQL would be
    an incomplete implementation of "DB-backed (dashboard-settable too)".
  • Deliberately did not thread aiReviewLowConfidenceDisposition through
    packages/gittensory-engine/src/predicted-gate.ts's own policy-building call (the MCP predict_gate
    tool's actual manifest→policy mapping) — this mirrors the exact same partial-wiring precedent already set
    by linkedIssueSatisfactionGateMode, which also isn't threaded through that predictor. The predictor's
    isConfiguredGateBlocker copy does receive the field (byte-correct for a future predictor-side change),
    it just isn't populated from a real .gittensory.yml read yet. Noted here rather than expanding scope.
  • Did not touch src/signals/settings-preview.ts's RepoSettingsPreview type — the sibling
    aiReviewCloseConfidence field is also absent from that preview type, and no automated check requires
    parity there (unlike RepositorySettingsSchema, which the ui:openapi:settings-parity script does
    enforce, and which this PR does update).

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally (unsharded) — all touched-file branches verified covered both ways via
    scoped coverage runs (src/rules/advisory.ts, src/settings/agent-actions.ts, src/db/repositories.ts,
    src/signals/focus-manifest.ts, src/api/routes.ts, src/queue/processors.ts); full-suite run green
    other than one pre-existing, unrelated flake (see Notes).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:openapi:settings-parity
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New/changed behavior has unit + integration tests for every new branch: test/unit/rules.test.ts,
    test/unit/agent-actions.test.ts, test/unit/gate-check-policy.test.ts, test/unit/focus-manifest.test.ts,
    test/unit/repository-settings-ai-review-low-confidence-disposition.test.ts (new),
    test/unit/routes-ai-byok.test.ts, test/unit/queue.test.ts (2 new end-to-end regressions proving the
    actual bug is fixed: a sub-floor cached defect holds instead of closing under the default, and closes
    under an explicit one_shot opt-in), and a new dedicated packages/gittensory-engine/test/gate-advisory- ai-review-low-confidence-disposition.test.ts for the engine twin.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, PATs, private keys, raw trust scores, private rankings,
    or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized (the new hold comment routes through sanitizePublicComment).
  • API/OpenAPI behavior is updated and tested (RepositorySettingsSchema, openapi.json, the /ai-review
    route, the internal full-replace route).
  • No visible UI change — no UI Evidence section needed.

Notes

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 10, 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 4afac01 Commit Preview URL

Branch Preview URL
Jul 10 2026, 10:06 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 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.12%. Comparing base (d571d80) to head (4afac01).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4656   +/-   ##
=======================================
  Coverage   94.12%   94.12%           
=======================================
  Files         434      434           
  Lines       38542    38565   +23     
  Branches    14047    14065   +18     
=======================================
+ Hits        36277    36300   +23     
  Misses       1604     1604           
  Partials      661      661           
Files with missing lines Coverage Δ
...es/gittensory-engine/src/advisory/gate-advisory.ts 100.00% <100.00%> (ø)
packages/gittensory-engine/src/focus-manifest.ts 99.11% <100.00%> (+<0.01%) ⬆️
src/api/routes.ts 94.26% <100.00%> (+<0.01%) ⬆️
src/db/repositories.ts 96.62% <100.00%> (+<0.01%) ⬆️
src/db/schema.ts 72.72% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)
src/queue/processors.ts 95.29% <100.00%> (+<0.01%) ⬆️
src/rules/advisory.ts 97.38% <100.00%> (+0.10%) ⬆️
src/services/ai-review.ts 96.59% <ø> (ø)
src/settings/agent-actions.ts 97.73% <100.00%> (+0.01%) ⬆️
... and 2 more
🚀 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 10, 2026
@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-10 09:55:06 UTC

25 files · 1 AI reviewer · no blockers · readiness 100/100 · CI failing · unstable

🛑 Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/queue/processors.ts (matched src/queue/**), src/rules/advisory.ts (matched src/rules/**), src/services/ai-review.ts (matched src/services/**), src/settings/agent-actions.ts (matched src/settings/agent-actions.ts).

Review summary
This PR resolves #4603 by wiring a new aiReviewLowConfidenceDisposition enum (one_shot | hold_for_review | advisory_only, default hold_for_review) through the previously dead aiReviewCloseConfidence floor in isConfiguredGateBlocker, and correctly threads it end-to-end: schema+migration+repository, routes+OpenAPI (regenerated), config-as-code docs (.gittensory.yml.example and full.yml kept in parity), the willClose narrowing in agent-actions.ts (only softens the conclusion==='failure' arm, leaving ciFailed/isConflict closes untouched), and the packages/gittensory-engine predictor twin. Tests in rules.test.ts and the two new queue.test.ts sweep-driven cases exercise the real cached-AI-review path and assert on actual gate_outcomes/audit_events rows rather than fabricated state, which is exactly what this repo's test-quality bar asks for. The one real gap is that codecov/patch failed at 83.33% against the 99% target — a meaningful shortfall, most plausibly from the untested '??' fallback arm in src/api/routes.ts where an omitted lowConfidenceDisposition falls back to current.aiReviewLowConfidenceDisposition.

Blockers

  • src/api/routes.ts's /ai-review PUT handler (`aiReviewLowConfidenceDisposition: parsed.data.lowConfidenceDisposition ?? current.aiReviewLowConfidenceDisposition`) and the full-settings PUT pass-through appear to be the source of the codecov/patch shortfall (83.33% vs 99% target) — only the explicit-value-set arm is covered by the visible routes-ai-byok.test.ts case, not the omitted-field/'keep current' arm.
Nits — 7 non-blocking
  • packages/gittensory-engine/src/advisory/gate-advisory.ts:46 redeclares an inline `"one_shot"|"hold_for_review"|"advisory_only"|undefined` union instead of importing the `AiReviewLowConfidenceDisposition` type this same PR adds to packages/gittensory-engine/src/types/manifest-deps-types.ts.
  • The predictor package's own isConfiguredGateBlocker comment admits aiReviewLowConfidenceDisposition isn't threaded through the predictor's policy-building call site yet ('same deliberate partial-wiring precedent as linkedIssueSatisfactionGateMode') — worth a one-line confirmation from the contributor that this is intentional and matches existing repo precedent rather than a new silent gap.
  • External secret-scan flagged test/unit/queue.test.ts:6353,6403 as possible generic secret assignments; these read as generatePrivateKeyPem()-style test fixtures rather than real leaked credentials, but I couldn't see the full file to fully rule it out.
  • Push patch coverage on src/api/routes.ts to the repo's ~97% branch-counted bar by adding a test for the omitted-lowConfidenceDisposition arm on both the /ai-review PUT route and the full-settings PUT route before merge.
  • Import AiReviewLowConfidenceDisposition in packages/gittensory-engine/src/advisory/gate-advisory.ts instead of re-declaring the union inline, for DRY consistency with the host copy in src/types.ts.
  • 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.

CI checks failing

  • codecov/patch — 83.33% of diff hit (target 99.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4603
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, 330 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 330 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The PR fixes all five stale comments plus isConfiguredGateBlocker/consensusDefectOf/runGittensoryAiReview return sites to be mutually consistent, records an explicit decision in the description (introducing a configurable aiReviewLowConfidenceDisposition defaulting to hold_for_review, which restores the downgrade-to-hold behavior the issue's resolution #2 asked for), and adds regression tests (tes

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), 330 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.
Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before / after /
/ mobile before / (mobile) after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 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

@JSONbored JSONbored self-assigned this Jul 10, 2026
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
…urable disposition (#4603)

isConfiguredGateBlocker voided aiReviewCloseConfidence entirely (commit 311b761, #1781), so a
sub-floor ai_consensus_defect/ai_review_split finding always one-shot-closed a contributor PR under
aiReviewGateMode: block regardless of the AI reviewers' own calibrated confidence, while five other
comments across the codebase still described the old (correct) "downgrades to a hold" behavior.

Adds aiReviewLowConfidenceDisposition ("one_shot" | "hold_for_review" | "advisory_only", default
hold_for_review) to govern a sub-floor finding: hold_for_review (shipped default) still blocks the
gate but routes the would-be close through the same held-for-manual-review mechanism the disposition
planner already uses for migrationCollisionHold/unlinkedIssueMatchHold; one_shot keeps today's
unconditional-close behavior as an opt-in; advisory_only drops a sub-floor finding to non-blocking.

Wires the new field through the full template this repo uses for a RepositorySettings field:
migration + Drizzle schema + resolver/parser (src/db/repositories.ts), OpenAPI schema + regenerated
apps/gittensory-ui/public/openapi.json, .gittensory.yml config-as-code override
(gate.aiReview.lowConfidenceDisposition), and the maintainer /ai-review + internal settings routes.
Mirrors the gate-decision change into the packages/gittensory-engine predictor twin so the MCP
predict_gate tool stays accurate. Fixes all 6 stale-comment locations the original audit found.
…ose codecov/patch gap on #4603

Two independent problems found in PR #4656 (#4603) before merge, both fixed here:

1. The PR silently deleted src/services/public-accuracy-trend.ts (181 lines) and its test,
   un-exported the public-stats.ts helpers it depended on, and dropped the accuracyTrend field
   from the public GET /v1/public/stats response -- entirely unrelated to #4603's actual scope
   (a live, wired-in public homepage feature from epic #4445/#4447, undisclosed in the commit
   message or PR body). Restored all four pieces verbatim from main.

2. codecov/patch was failing (83.33% vs 99% target): the new isConfiguredGateBlocker branches
   for aiReviewLowConfidenceDisposition in packages/gittensory-engine/src/advisory/gate-advisory.ts
   had a dedicated node:test suite, but that suite runs against the compiled dist/ output in a
   separate test runner whose coverage never reaches the coverage/lcov.info file Codecov actually
   reads -- only the root vitest suite's coverage does, and it only touches this file through the
   handful of test/unit/*.ts files that import it directly by relative path (not via the npm
   package). Added the missing branch cases to the existing
   test/unit/predicted-gate-engine-branch-coverage.test.ts, which does import it that way.

   Also found src/api/routes.ts's `updated.aiReviewLowConfidenceDisposition ?? "hold_for_review"`
   fallback is unreachable in practice -- parseAiReviewLowConfidenceDisposition's return type is
   non-nullable and already falls back to the same literal itself -- so marked it
   /* v8 ignore next */ rather than writing a contrived test for a branch that can't actually run.

Part of #4603
@JSONbored
JSONbored force-pushed the fix/ai-review-low-confidence-disposition-4603 branch from 5c6a8e0 to 4afac01 Compare July 10, 2026 10:04
@JSONbored

Copy link
Copy Markdown
Owner Author

Independent review before merge found two problems, both fixed in the latest push:

1. Undisclosed, out-of-scope deletion. The original commit silently removed src/services/public-accuracy-trend.ts (a live, wired-in public feature from epic #4445/#4447 — the homepage's weekly "Decision accuracy %" trend on GET /v1/public/stats), its test, and un-exported the public-stats.ts helpers it depended on. None of this was mentioned in the commit message or PR body, and it's entirely unrelated to #4603's scope. Restored all of it verbatim from main.

2. codecov/patch was failing (83.33% vs 99% target). The new isConfiguredGateBlocker branches in packages/gittensory-engine/src/advisory/gate-advisory.ts had a dedicated node:test suite, but that suite runs against compiled dist/ output in a separate test runner whose coverage never reaches coverage/lcov.info — only the root vitest suite's coverage does, via the handful of test/unit/*.ts files that import the engine source directly by relative path. Added the missing cases to test/unit/predicted-gate-engine-branch-coverage.test.ts, which does import it that way — confirmed 100% branch coverage on the new logic. Also found routes.ts's updated.aiReviewLowConfidenceDisposition ?? "hold_for_review" fallback is unreachable in practice (parseAiReviewLowConfidenceDisposition's return type is non-nullable and already defaults to the same literal), so marked it /* v8 ignore next */ instead of writing a test for a branch that can't run.

Full local run of every touched test file (11 files, 1952 tests) passes; typecheck clean.

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

Development

Successfully merging this pull request may close these issues.

Resolve dead aiReviewCloseConfidence floor (decision + fix)

1 participant