Bolt 2: 品質修復Loop Pluginを追加する - #2194
Conversation
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughQuality Repairの品質証拠処理、再計画、停止・再開、永続化、監査リプレイを追加しました。Loop MonitorのJudgeルート制約と状態更新を強化し、監査イベント件数を87件へ更新しました。 ChangesQuality Repair
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/framework/core/tools/amadeus-loop-monitor.ts`:
- Around line 762-763: Update duplicate-delivery detection in the relevant
monitor flow around isValidJudgeRouteConstraint so an existing delivery is
considered identical only when deliveryId, payloadFingerprint, and
routeConstraint.fingerprint all match. Return CONFLICT for matching payloads
with different route constraints, and add a regression test covering a
full-route constraint followed by a subset constraint.
In `@packages/framework/core/tools/amadeus-quality-repair-replay.ts`:
- Around line 57-68: Strengthen the type-guard validation in
packages/framework/core/tools/amadeus-quality-repair-replay.ts#L57-L68 by
requiring latestSnapshot.unresolved, latestSnapshot.verifierSuccessReceipts, and
stalledLatch.unresolvedObligationIds to be arrays, and
pendingReplan.attemptIdentity to be a string. Also update the loopEventSets
validation at
packages/framework/core/tools/amadeus-quality-repair-replay.ts#L98-L99 so every
element’s partitionKey is a string; keep validProjection and validTransaction
fail-closed at the same nested depth.
- Around line 98-99: Update the loopEventSets validation in the replay input
check to also require set.partitionKey to be a string, alongside the existing
eventSetId and events checks. Use the validation consumed by
createMemoryQualityRepairRepository and loopReceipt so malformed sets cannot
reach allLoopSets or receipt generation without a partition key.
In `@packages/framework/core/tools/amadeus-quality-repair-runtime.ts`:
- Around line 806-816: Update the evidence-change resume path that constructs
QualityRuntimeProjection so the validated normalized.snapshot is retained in
latestSnapshot instead of being reset to null, while preserving the existing
reset behavior for lastProgress and other transient fields. Ensure
projectQualityRepairStatus can immediately report the resumed snapshot’s
obligations and evidenceFingerprint without requiring recordEvidence again.
- Around line 492-501: Cache the result of
foldQualityProjections(repository.readTransactions()) and invalidate that cache
only when a transaction is committed, so readProjection, readByPartition,
recordEvidence, and dispatchJudge do not refold the full history on every call.
Maintain a partitionKey-to-qualityScopeId index alongside the cached projections
and update readByPartition to use the index instead of scanning
projections.values().
- Around line 205-210: Update the loop-only transaction result handling around
`result` and `repository.transaction` to use a distinct sentinel for “body was
not called” rather than initializing with `undefined`. After the transaction,
detect the sentinel explicitly and handle that state separately, while returning
the body’s value—including a legitimate `undefined`—without an unchecked cast.
- Around line 429-433: The loopMonitors array access uses a non-null assertion
operator but the type definition does not express that the array is guaranteed
to be non-empty. Either update the type definition of loopMonitors in
CompiledLoopMonitorGraph to use a non-empty array type like readonly
[CompiledLoopMonitor, ...CompiledLoopMonitor[]], or add an explicit validation
at the entry point of createQualityRepairCoordinator that checks
activation.graph.loopMonitors.length === 0 and throws an error with an
appropriate reason code if the array is empty, then remove the non-null
assertions at the access sites around line 429 and line 490.
In `@packages/framework/core/tools/amadeus-quality-repair.ts`:
- Around line 726-769: When `snapshot.unresolved.length === 0` (indicating all
quality defects are resolved), the current logic does not recognize this as a
convergence state and instead increments `consecutiveNonProgress`, incorrectly
triggering repair-stalled transitions. Add a guard condition after the `window`
calculation in the quality epoch projection logic to detect when unresolved is
empty, and in that case return a progress state of kind "strict-progress" with
`consecutiveNonProgress: 0`, `replanSinceLastProgress: false`,
`deterministicAction: null`, and `routeIds: []`. Then add a test case to
`packages/framework/core/tools/amadeus-quality-repair.ts#L726-L769` anchor site
that verifies this behavior by injecting consecutive snapshots with empty
unresolved lists, confirming the counter stays at 0 and no repair-stalled
transition occurs, and add corresponding test assertions at
`tests/unit/t428-quality-repair.test.ts#L337-L356` sibling site.
- Around line 464-523: 各観測種別の obligationId 導出を共通ヘルパーに集約し、obligation() と
sensor/produce/condition 分岐の双方からそのヘルパーを利用してください。sensor の blocking、produce の
required など不要な観測では早期 return を先に行い、success を生成する直前にのみ obligationIdOf
相当の導出を実行するよう計算を遅延させてください。
- Around line 430-435: Update the reviewer-observation handling around the READY
verdict check so a READY observation with any blockers is rejected as
evidence-incomplete. Validate blockers before returning the empty
obligations/successes result, while preserving the current empty result for
READY observations without blockers.
- Around line 158-172: The transitionTable declares a transition from
QUALITY_CHECK to QUALITY_REPLAN, but this state is unreachable since
planQualityDelivery never issues QUALITY_REPLAN events, and no routes target it.
Additionally, the repair and replan routes in the routes array both have
identical targetEvent values, making them indistinguishable at runtime. Either
remove QUALITY_REPLAN from the transitionTable and its outgoing transition from
QUALITY_CHECK, or alternatively change the replan route's targetEvent from
QUALITY_NON_PROGRESS to QUALITY_REPLAN and ensure planQualityDelivery can emit
QUALITY_REPLAN events to make the state reachable and the two routes
distinguishable.
In `@tests/integration/t429-quality-repair-runtime.integration.test.ts`:
- Line 239: Replace the three type-narrowing early returns after expect in
tests/integration/t429-quality-repair-runtime.integration.test.ts at lines 239,
251, and 318 with throws using the specified messages: “missing first
threshold”, “missing stalled threshold”, and “missing resumed projection”,
respectively. Keep the existing expect assertions and use throw new Error so
each guard cannot silently complete the test.
In
`@tests/integration/t430-quality-repair-five-harness-projection.integration.test.ts`:
- Around line 21-27: The currentHarnessCohort array contains redundant dir
property definitions that duplicate information already available in the
manifest objects. Remove the manual dir mappings like ".claude", ".codex", etc.
and instead derive the directory names from the manifest.harnessDir property of
each manifest object (claudeManifest, codexManifest, cursorManifest,
opencodeManifest, kimiManifest) to eliminate duplication and maintain a single
source of truth.
In `@tests/unit/t428-quality-repair.test.ts`:
- Around line 216-224: Add a test case to verify that normalizeQualityEvidence
handles the "NOT-READY" notation (with hyphen) in addition to the existing "NOT
READY" test. Create a second incomplete reviewer test scenario identical to the
current test but with verdict set to "NOT-READY", and verify it produces the
same expected output with ok: true and the same evidence-incomplete snapshot,
ensuring both verdict notations generate consistent obligations.
- Around line 66-110: Add missing test cases to cover the fail-closed paths for
quality contribution validation in the test file. The current tests only
validate the contribution: null path, but compileQualityContribution has five
rejection conditions that need coverage. Add three new test cases: one verifying
that a tampered contentDigest in the contribution object triggers
ACTIVATION_FAILED when calling resolveQualityPluginActivation, another checking
that a repair-stalled route rule with disposition set to "continue" also
triggers ACTIVATION_FAILED, and a third confirming that
planNoneModeQualitySetting with the opted-out flag set to false returns
QUALITY_REPAIR_OPTED_OUT error with provenanceTurnId: null. These additions
ensure Issue `#2096`'s acceptance criteria of fail-closed behavior during
preflight validation for missing or corrupted contributions are properly
verified.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d220df7d-ab2f-4b15-9eb0-38d8f0bd117e
⛔ Files ignored due to path filters (2)
amadeus/spaces/default/intents/260803-intent-autonomy/construction/quality-repair-runtime/code-generation/code-generation-plan.mdis excluded by!amadeus/spaces/*/intents/**amadeus/spaces/default/intents/260803-intent-autonomy/construction/quality-repair-runtime/code-generation/code-summary.mdis excluded by!amadeus/spaces/*/intents/**
📒 Files selected for processing (18)
docs/reference/12-state-machine.mdpackages/framework/core/knowledge/amadeus-shared/audit-format.mdpackages/framework/core/otel/event-registry.tspackages/framework/core/tools/amadeus-audit.tspackages/framework/core/tools/amadeus-loop-monitor-runtime.tspackages/framework/core/tools/amadeus-loop-monitor.tspackages/framework/core/tools/amadeus-quality-repair-replay.tspackages/framework/core/tools/amadeus-quality-repair-runtime.tspackages/framework/core/tools/amadeus-quality-repair.tstests/.coverage-ratchet.jsontests/.coverage-registry.jsontests/integration/event-registry-drift.test.tstests/integration/t429-quality-repair-runtime.integration.test.tstests/integration/t430-quality-repair-five-harness-projection.integration.test.tstests/unit/t28-audit-event-sync.test.tstests/unit/t426-loop-monitor.test.tstests/unit/t428-quality-repair.test.tstests/unit/t81.test.ts
The self-hosted patch coverage gate flagged 54 added lines with zero hits. Reach the measurable ones with new tests and allowlist the rest: - cover the duplicate-route constraint rejection and the invalid route-constraint catch path in the loop monitor reducer (t426) - cover createAuditQualityRepairRepository commit/reload round-trip through the audit ledger, including the guarded emit and audit-lock seams (t429) - cover the fail-closed CONFLICT arm when an evidence resume cannot commit its loop delivery (t429) - allowlist type-only multiline signature/annotation lines that Bun reports as zero-hit DA records only in the merged suite, following the existing goal-reconciliation precedent - allowlist the defensive re-normalization failure branch in rebaseResumeEvidence, unreachable while resolveResumeBasis validates the identical evidence first
The merged-suite coverage run still emits zero-hit DA records for the multiline input and return type annotations of createAuditQualityRepairRepository and replayQualityRepairScope even though both functions are now exercised by t429 integration tests. Follow the established type-only allowlist precedent for them.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/framework/core/tools/amadeus-quality-repair-runtime.ts (1)
903-913: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
latestSnapshotとlastProgressの型契約が崩れます。
resumePlanはplanQualityDeliveryの戻り値です。戻り値はQualityDeliveryPlanインターフェースで、常に非 null のprogressフィールドを持ちます。resumedSnapshotが非 null の場合、resumePlanも非 null で、progressを含みます。しかし、現在のコードは
lastProgress: nullに設定します。これにより、stale previousSnapshot 経路(708-717行)でprogress: priorByInput.lastProgress!を返す際に、非 null assertion は null 値を返します。呼び出し側がprogress.kindを参照するとランタイムエラーが発生します。
resumePlan?.progress ?? nullに修正してください。resumedSnapshotが非 null の場合、resumePlan.progressの値を使用します。null の場合は null に設定されます。修正案
epoch: nextEpoch, latestSnapshot: resumedSnapshot, - lastProgress: null, + lastProgress: resumePlan?.progress ?? null, observationSequence: 0,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/framework/core/tools/amadeus-quality-repair-runtime.ts` around lines 903 - 913, Update the projection construction around QualityRuntimeProjection to set lastProgress from resumePlan?.progress ?? null instead of always null. Preserve null when no resume plan exists, and use the non-null QualityDeliveryPlan.progress whenever resumedSnapshot is available so the existing stale previousSnapshot path returns a valid progress value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/framework/core/tools/amadeus-quality-repair-replay.ts`:
- Around line 99-106: Replace the local validLoopEventSet implementation with
the exported validEventSet from amadeus-loop-monitor-replay. Update
decodeQualityRepairTransaction to pass its already-parsed value directly to
validEventSet, removing the JSON.stringify and decodeLoopMonitorEventSet round
trip while preserving the existing validation behavior.
---
Outside diff comments:
In `@packages/framework/core/tools/amadeus-quality-repair-runtime.ts`:
- Around line 903-913: Update the projection construction around
QualityRuntimeProjection to set lastProgress from resumePlan?.progress ?? null
instead of always null. Preserve null when no resume plan exists, and use the
non-null QualityDeliveryPlan.progress whenever resumedSnapshot is available so
the existing stale previousSnapshot path returns a valid progress value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60172ccf-9c41-4884-8ef7-dae2db904283
📒 Files selected for processing (14)
packages/framework/core/tools/amadeus-loop-monitor-runtime.tspackages/framework/core/tools/amadeus-loop-monitor.tspackages/framework/core/tools/amadeus-quality-repair-replay.tspackages/framework/core/tools/amadeus-quality-repair-runtime.tspackages/framework/core/tools/amadeus-quality-repair.tstests/.coverage-patch-allowlist.jsontests/integration/t429-quality-repair-runtime.integration.test.tstests/integration/t430-quality-repair-five-harness-projection.integration.test.tstests/no-silent-drop/baseline.jsontests/no-silent-drop/exemptions.jsontests/unit/t426-loop-monitor.test.tstests/unit/t427-loop-monitor-runtime.test.tstests/unit/t428-quality-repair.test.tstests/unit/t429-quality-repair-replay-validation.test.ts
Address CodeRabbit review on #2194: decodeQualityRepairTransaction already validates a JSON.parse result, so the loop event set check no longer round-trips through JSON.stringify + decodeLoopMonitorEventSet. Export validEventSet from the loop replay module and call it directly; validation semantics are unchanged.
Bolt 2 (U2 quality-repair-runtime, issue #2096) was re-delivered on the delivery plan via PR #2194 and merged to main as 04a5b39. Move the parked workflow's Session Resume Point from Bolt 2 to Bolt 3 so the next resume continues the recovery at U3 intent-autonomy-runtime. The workflow stays parked at build-and-test until that session starts.
概要
Delivery PlanのBolt 2 / U2
quality-repair-runtimeを実装します。マージ済みのBolt 1(#2181)の汎用Loop Monitorを利用し、品質不備を正規化してrepair / replan / repair-stalledへ閉じた形で分岐させます。主な変更
REPAIR_STALLEDを実装スコープ境界
検証
bun run build: 成功Closes #2096
Note
Medium Risk
Introduces durable workflow state (replan, latch, suspend/resume) and a new canonical audit event, but reuses the existing Loop Monitor coordinator and is covered by extensive unit/integration and registry drift guards.
Overview
Adds Bolt 2 / U2
quality-repair-runtime: a first-party quality repair loop on top of the generic Loop Monitor, without quality-specific vocabulary in the monitor core.New runtime surface:
amadeus-quality-repair.ts(embedded contribution, activation, blocking-evidence → obligations/snapshots, T+1 convergence, replan-first / post-replanrepair-stalled),amadeus-quality-repair-runtime.ts(atomic quality + loop transactions, Judge/replan attempts 0/1,REPAIR_STALLED/ suspended workflow, evidence-or-human resume), andamadeus-quality-repair-replay.ts(audit decode, repository, replay).Loop Monitor seam (minimal): deliveries may carry a validated non-empty subset of compiled routes into Judge reservations and dispatch;
LOOP_ROUTE_APPLIEDcan advance cycle/epoch when a transition target is present.Audit & docs: registers
QUALITY_REPAIR_TRANSACTION_COMMITTED(canonical count 86 → 87), updates state-machine reference, coverage registry/ratchet, and drift tests.Tests: unit/integration for convergence, stall/resume, replay conflicts, and a five-harness check that projected quality runtime files match core bytes.
Reviewed by Cursor Bugbot for commit 927822b. Configure here.
Summary by CodeRabbit
新機能
ドキュメント