feat(u3-question-route-observability): QUESTION_ANSWERED に解決経路の観測属性を追加する - #2487
Merged
Conversation
… route on QUESTION_ANSWERED Add a derived Resolution Route attribute (ladder when --decision-id is passed, human otherwise) plus an optional Decision Id to the QUESTION_ANSWERED emit point in amadeus-log.ts, so decide-question ladder answers and direct human answers are machine-discriminable in the audit shard (FR-3a, #2378). Export questionAnswerRouteRows / findBypassedQuestionAnswers, an after-the-fact sweep that derives the Intent autonomy mode from INTENT_AUTONOMY_TRANSACTION_COMMITTED rows and flags human answers recorded under semi/full mode (FR-3b). Observation only: the sole new refusal is the auto-decision- shape check on an explicitly passed --decision-id (FR-3c); pre-u3 rows read as route unknown and existing checkpoint guards are pinned unchanged. Tests: t487 (unit, pure derivation + predicate with a real semi-mode transaction fixture and a falling-proof route rewrite) and t486 (integration, both routes end to end via the exported main seam, loud malformed-id refusal, open-gate guard contrast, shard-level detection with fixture-rewrite falling proof).
…ep catch and rebind the approval census
📝 WalkthroughWalkthrough
Changes質問回答ルートの観測
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…ler onto its guarded call error() exits the process, so the malformed --decision-id arm is only reachable from a spawned run and the patch census counted it as an uncovered added line. Sharing the line with the call it guards keeps the behaviour identical while the executed assignment carries the record.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/knowledge/amadeus-shared/audit-format.md`:
- Line 150: QUESTION_ANSWERED のオプション属性一覧に、event-registry.ts の許可内容と一致する Presence
Reservation Id を追加してください。既存の属性および amadeus-log.ts answer の記載は変更しないでください。
In `@tests/unit/t487-question-route-derivation.test.ts`:
- Around line 103-159: テストに、正常な semi または full の autonomy 行の後へ復号不能な Transaction
行を追加するケースを補ってください。questionAnswerRouteRows では復号失敗後も直前の autonomyMode
が保持されることを検証し、findBypassedQuestionAnswers ではその後の human route が保持された mode
に基づいて期待どおり検出されることを確認してください。
🪄 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: 7d70f4ab-3432-452e-9b1c-44f9727891c9
📒 Files selected for processing (10)
packages/framework/core/knowledge/amadeus-shared/audit-format.mdpackages/framework/core/otel/event-registry.tspackages/framework/core/tools/amadeus-log.tstests/.coverage-registry.jsontests/integration/t-coverage-mechanism-ratchet.test.tstests/integration/t413-no-silent-drop-ci-adoption.test.tstests/integration/t486-question-route-observability.integration.test.tstests/no-silent-drop/approval.jsontests/no-silent-drop/events/01KZF9NRBGKJZKY9TGJAJYQ45G.jsontests/unit/t487-question-route-derivation.test.ts
…ion row and full mode Review follow-up. The sweep's catch — an undecodable transaction keeps the last readable mode — and the predicate's full-mode arm had no test; both now stand on a real transaction minted through the coordinator.
u1 landed the sibling autonomy waiver, so the shared ledgers take the union of both: the approval census carries 215 entries (213 base plus one grant from each unit), the mechanism ratchet lists all four new spawner tests, and t413's pin follows the census.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/framework/core/knowledge/amadeus-shared/audit-format.md (1)
281-281: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winイベント数の見出しを
6に更新してください。
INTENT_AUTONOMY_HUMAN_REQUIREDの追加後、この表は6件です。見出しは5 eventsのままです。監査イベント一覧の件数を実際の行数と一致させてください。🤖 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/knowledge/amadeus-shared/audit-format.md` at line 281, 監査イベント一覧の見出しを `5 events` から `6 events` に更新し、`INTENT_AUTONOMY_HUMAN_REQUIRED` を含む表の行数と一致させてください。tests/.coverage-registry.json (1)
331-337: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win監査ユニットのカバレッジ登録を更新してください。
INTENT_AUTONOMY_HUMAN_REQUIREDにt482-autonomy-refusal-event.integration.test.ts（cli）を追加し、statusをcoveredにする。QUESTION_ANSWEREDにt486-question-route-observability.integration.test.ts（cli）を追加し、statusをcoveredにする。counts.coveredByClass.auditを 44 から 46 に更新する。🤖 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 `@tests/.coverage-registry.json` around lines 331 - 337, Update the audit coverage registry entries for INTENT_AUTONOMY_HUMAN_REQUIRED and QUESTION_ANSWERED by adding their specified CLI integration tests to coveredBy and changing each status to covered; then update counts.coveredByClass.audit from 44 to 46.
🤖 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.
Outside diff comments:
In `@packages/framework/core/knowledge/amadeus-shared/audit-format.md`:
- Line 281: 監査イベント一覧の見出しを `5 events` から `6 events`
に更新し、`INTENT_AUTONOMY_HUMAN_REQUIRED` を含む表の行数と一致させてください。
In `@tests/.coverage-registry.json`:
- Around line 331-337: Update the audit coverage registry entries for
INTENT_AUTONOMY_HUMAN_REQUIRED and QUESTION_ANSWERED by adding their specified
CLI integration tests to coveredBy and changing each status to covered; then
update counts.coveredByClass.audit from 44 to 46.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a83ecf2b-d190-41b0-b570-597897092eb5
📒 Files selected for processing (7)
packages/framework/core/knowledge/amadeus-shared/audit-format.mdpackages/framework/core/otel/event-registry.tstests/.coverage-registry.jsontests/integration/t-coverage-mechanism-ratchet.test.tstests/integration/t413-no-silent-drop-ci-adoption.test.tstests/no-silent-drop/approval.jsontests/unit/t487-question-route-derivation.test.ts
main took t486 and t487 while this branch was in review, so the two tests here move up to t488/t489 and every reference follows. The mechanism ratchet lists both sides' spawners.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/.coverage-registry.json`:
- Line 6235: Update the QUESTION_ANSWERED audit entry in
tests/.coverage-registry.json, not just the subcommand registration for
t488-question-route-observability.integration.test.ts. Add that integration test
to QUESTION_ANSWERED.coveredBy and change its status from UNCOVERED to covered,
preserving the existing audit coverage structure.
🪄 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: 4f17c6ff-e702-4dfb-b841-a8aeb52ec61d
📒 Files selected for processing (4)
tests/.coverage-registry.jsontests/integration/t-coverage-mechanism-ratchet.test.tstests/integration/t488-question-route-observability.integration.test.tstests/unit/t489-question-route-derivation.test.ts
| "minMechanism": "cli", | ||
| "coveredBy": [ | ||
| { | ||
| "file": "tests/integration/t488-question-route-observability.integration.test.ts", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
QUESTION_ANSWERED の監査単位も更新してください。
tests/integration/t488-question-route-observability.integration.test.ts は QUESTION_ANSWERED を生成します。現在の QUESTION_ANSWERED エントリは Line [519]-[523] で coveredBy: [] と UNCOVERED のままです。Line [6235] のサブコマンド登録だけでは、監査イベントのカバレッジ証跡が更新されません。
同じテストを QUESTION_ANSWERED.coveredBy に追加し、実際の機構に合わせて status を covered に更新してください。
修正例
"unitId": "QUESTION_ANSWERED",
"coveredBy": [
+ {
+ "file": "tests/integration/t488-question-route-observability.integration.test.ts",
+ "mechanism": "cli"
+ }
],
- "status": "UNCOVERED"
+ "status": "covered"🤖 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 `@tests/.coverage-registry.json` at line 6235, Update the QUESTION_ANSWERED
audit entry in tests/.coverage-registry.json, not just the subcommand
registration for t488-question-route-observability.integration.test.ts. Add that
integration test to QUESTION_ANSWERED.coveredBy and change its status from
UNCOVERED to covered, preserving the existing audit coverage structure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Intent 260807-autonomy-reachability の unit u3-question-route-observability —
QUESTION_ANSWERED監査イベントへ質問の解決経路(route)の観測属性を追加する。FR 要約
--decision-idからの導出属性である。呼び出し元が auto-decision id を渡した場合のみauto系 route が導出され、渡さない場合は従来どおりの human route として記録される。既存呼び出し元への必須フラグ追加はない。--decision-idのauto-decision-形式検査であり、malformed id は exit 1 で loud に停止しQUESTION_ANSWEREDを emit しない(t486 で落ちる実証を固定)。迂回述語(route=human × autonomy=semi/full の組)は観測専用で、検出しても停止しない。INTENT_AUTONOMY_TRANSACTION_COMMITTED行があっても読取全体を fail させず最後の既知 mode を維持する — 迂回質問の過剰検出側に倒す(t486/t487 でピン)。この observation-only catch は no-silent-drop 台帳へ grant イベント(01KZF9NRBGKJZKY9TGJAJYQ45G、NSD001、enhancement(engine): Intent autonomy full でも人間裁定が残る構造要因を計測した — 人間ターンの74%はモード設定前、presence guard 拒否231件/63 intents #2378)として登録し、approval census を 213 → 214 へ再束縛した。QUESTION_ANSWERED行の route はunknownとして読まれる。検証結果(いずれも exit code を個別捕捉)
bun run typecheckbun run lintbun test(t487 / t486 / no-silent-drop-gate / no-silent-drop-repository-adoption / t413 / t-coverage-mechanism-ratchet / event-registry-drift、--timeout=60000)bun tests/no-silent-drop-gate.ts approve-evidencebun tests/gen-coverage-registry.ts --checkbun run build後の追跡ファイル不変(git status --short= 台帳変更のみ)census 実測:
counts = { C_pre: 214, B_pre: 214, B0: 214 }/censusDigest = f89c1bcd9e3999c8d801a110df18d2b1b7e66286c7ed871dca5e0f6631b8c175レビュー
Refs #2378
Note
Low Risk
Observability-only audit attributes and read-side predicates; existing answer flows unchanged unless callers pass
--decision-id. The new malformed-id check is narrow and loud-fails without emitting.Overview
Adds u3 question-route observability so
QUESTION_ANSWEREDrows record how an answer was resolved:Resolution Route(humanvsladder) and optionalDecision Id, derived from whetheramadeus-log answerreceives a validauto-decision-*--decision-id(no new required flags for existing callers).amadeus-loggains route derivation (resolveQuestionRoute), an audit sweep (questionAnswerRouteRows/findBypassedQuestionAnswers) that flags direct human answers under semi/full autonomy, and treats pre-u3 rows without route asunknown. Malformed--decision-idis the only new refusal path. The observation-only sweep keeps the last known autonomy mode when anINTENT_AUTONOMY_TRANSACTION_COMMITTEDrow cannot be decoded (grandfathered in no-silent-drop).Docs/registry (
audit-format.md,event-registry.ts), t486/t487 tests, coverage ratchet, and NSD census 213 → 214 are updated accordingly.Reviewed by Cursor Bugbot for commit 5599e20. Configure here.
Summary by CodeRabbit