145 lines
8.1 KiB
Markdown
145 lines
8.1 KiB
Markdown
# diagnosis-eval-demo-gatekeeper-closure Decisions
|
|
|
|
## Clarify
|
|
|
|
- Entry summary: implement the next three interview-readiness items together: diagnosis eval fixture matrix, stable demo data set, and Gatekeeper rule configuration/audit version.
|
|
- Slug: `diagnosis-eval-demo-gatekeeper-closure`
|
|
- Devflow scale: `standard`
|
|
- Interface impact: expected L2 internal contract change because `gatekeeper_result` audit JSON will gain rule metadata/version fields.
|
|
|
|
## Context
|
|
|
|
- `devflow/index.md` used: related entries found for diagnosis eval harness, fixture expansion, MVP demo runbook, Gatekeeper hook, and verifier evidence reference fidelity.
|
|
- Relevant glossary:
|
|
- Evidence Tools produce incident facts and must be recorded in `tool_invocation`.
|
|
- Verifier should not use skills/runbooks as incident evidence.
|
|
- `tool_invocation.retrieval_details` is the structured evidence/audit home for tool-specific details.
|
|
- Historical constraints that must enter OpenSpec:
|
|
- Diagnosis eval is offline and deterministic; no LLM-as-judge.
|
|
- Demo assets should be runnable, but fixed regression should use saved fixtures.
|
|
- Gatekeeper remains in the Verifier hook path.
|
|
- No new database table for Gatekeeper audit; use `self_evaluation.verifier_evaluation.gatekeeper_result`.
|
|
- `$.no_evidence` is a query no-hit signal, not proof that a problem is impossible.
|
|
|
|
## Question Pool
|
|
|
|
| ID | Dimension | Mode | Question | Status |
|
|
|---|---|---|---|---|
|
|
| Q1 | Terminology | evidence-driven | What names should this change use for the matrix, demo set, and Gatekeeper rule metadata? | Resolved |
|
|
| Q2 | Boundary | evidence-driven | Should this change alter public APIs, database schema, Planner output, or retry behavior? | Resolved |
|
|
| Q3 | Acceptance | evidence-driven | Which existing tests and baseline assets define the current acceptance style? | Resolved |
|
|
| Q4 | Technical | evidence-driven | Where should Gatekeeper rule metadata live with minimal implementation risk? | Pending code research |
|
|
| Q5 | Scope | user-interview | Should the stable demo set be documentation/payloads only, or should it include live E2E scripts for all scenarios? | Confirmed |
|
|
|
|
## Evidence-driven Conclusions
|
|
|
|
- Q1 conclusion: use `diagnosis eval matrix`, `stable demo scenarios`, and `Gatekeeper rule set version` as terms.
|
|
- Q2 conclusion: keep this as an internal contract change. Do not add public endpoints, tables, Planner `scope_contract`, or Gatekeeper retry.
|
|
- Q3 conclusion: existing `DiagnosisTraceEvaluatorTest`, `ExecutorGatekeeperServiceTest`, `VerifierInputHookTest`, `ToolInvocationRecorderTest`, and `mvp/eval/reports` define the current acceptance style.
|
|
- Q4 conclusion: Gatekeeper metadata should live behind a small rule catalog loaded by `ExecutorGatekeeperService`; the audit output should include a rule set version and enabled rule metadata summary, without adding tables or remote registry.
|
|
|
|
## User-interview Confirmations
|
|
|
|
- Q5 confirmed by resumed objective: complete items 1/2/3 with sm-flow, archive, submit, and run end-to-end if necessary.
|
|
- Implementation interpretation: stable demo scenarios will be fixed request payloads and runbook docs plus deterministic fixture-backed eval. Live E2E remains necessary only for at least one main path or where unit/fixture evidence is insufficient.
|
|
|
|
## OpenSpec Backfill
|
|
|
|
- Created Draft proposal at `openspec/changes/diagnosis-eval-demo-gatekeeper-closure/proposal.md`.
|
|
- Context constraints from historical devflow entries were written into the proposal.
|
|
- Scope confirmation and Gatekeeper catalog placement were written into the proposal/design.
|
|
|
|
## Current Checkpoint
|
|
|
|
- Discover completed.
|
|
- No implementation files changed yet.
|
|
|
|
## Specify / Alignment
|
|
|
|
### Cross-artifact Alignment
|
|
|
|
| Check | Status | Notes |
|
|
|---|---|---|
|
|
| brief/proposal goals -> proposal | Aligned | Proposal covers eval matrix, stable demo scenarios, and Gatekeeper rule catalog/audit version. |
|
|
| proposal scope/constraints -> design | Aligned | Design records offline deterministic eval, fixture-backed demo distinction, local rule catalog, and no new table/API. |
|
|
| design decisions -> specs/tasks | Aligned | Specs cover eval matrix, rule set version validation, demo scenarios, and Gatekeeper rule metadata; tasks cover matching implementation slices. |
|
|
| specs observable behavior -> tasks | Aligned | Each requirement has an executable task and acceptance check. |
|
|
|
|
### Interface Impact
|
|
|
|
- Level: L2 internal contract change.
|
|
- Reason: `gatekeeper_result` internal audit JSON gains `rule_set_version` and rule metadata summary. Eval case/result fields may gain optional rule set checks. No public HTTP API, database schema, or external DTO contract changes.
|
|
|
|
## Audit
|
|
|
|
Input -> processing -> output chain:
|
|
|
|
```text
|
|
mvp/demo request docs + mvp/eval fixtures
|
|
-> DiagnosisTraceEvaluator
|
|
-> baseline reports
|
|
-> interview/demo evidence
|
|
|
|
Gatekeeper rule catalog
|
|
-> ExecutorGatekeeperService
|
|
-> VerifierInputHook / ChatService persisted self_evaluation
|
|
-> Trace and eval audit
|
|
```
|
|
|
|
Architecture risk assessment:
|
|
|
|
1. The change is intentionally internal and should not add new public consumers.
|
|
2. Gatekeeper catalog must stay metadata-only; dynamic rule execution would be a different, riskier architecture.
|
|
3. Fixture-backed demo scenarios should be documented as deterministic regression artifacts, not live LLM guarantees.
|
|
4. Baseline report churn is expected and must be committed with case/fixture changes.
|
|
5. No devflow/OpenSpec conflict found.
|
|
|
|
## Commit Gate
|
|
|
|
- `cmd /c openspec validate diagnosis-eval-demo-gatekeeper-closure --strict`: passed.
|
|
- `cmd /c openspec validate --specs`: passed, 10 specs passed.
|
|
- File completeness:
|
|
- proposal.md: present.
|
|
- design.md: present.
|
|
- specs: present for `diagnosis-eval-harness`, `mvp-demo-trace-acceptance`, `chat-verifier-agent`.
|
|
- tasks.md: present.
|
|
- Consistency:
|
|
- Proposal concepts have corresponding design sections.
|
|
- Design decisions are reflected in specs/tasks.
|
|
- Task acceptance checks are verifiable.
|
|
|
|
## Current Checkpoint
|
|
|
|
- Commit completed.
|
|
- `.committed` marker created.
|
|
|
|
## Apply Verification
|
|
|
|
- Focused verification passed:
|
|
- `mvn "-Dtest=ExecutorGatekeeperServiceTest,DiagnosisTraceEvaluatorTest,DiagnosisEvalBaselineDiffTest,VerifierInputHookTest" test`
|
|
- Result: 36 tests, 0 failures, 0 errors.
|
|
- Broader relevant regression passed:
|
|
- `mvn "-Dtest=DiagnosisTraceEvaluatorTest,DiagnosisEvalBaselineDiffTest,ExecutorGatekeeperServiceTest,VerifierInputHookTest,ChatServiceSequentialAgentTest,ToolInvocationRecorderTest,QueryLogsToolsTest" test`
|
|
- Result: 61 tests, 0 failures, 0 errors.
|
|
- E2E startup repro found a Spring bean construction issue:
|
|
- Command: `mvn spring-boot:run "-Dspring-boot.run.profiles=mvp-demo"`
|
|
- Failure: `ExecutorGatekeeperService` had two public constructors and no annotated constructor, so Spring attempted a no-arg constructor and failed with `No default constructor found`.
|
|
- Classification: code deviation from OpenSpec implementation intent, not a spec gap.
|
|
- Fix: annotate the production constructor with `@Autowired`.
|
|
- Post-fix focused regression passed:
|
|
- `mvn "-Dtest=ExecutorGatekeeperServiceTest,VerifierInputHookTest" test`
|
|
- Result: 23 tests, 0 failures, 0 errors.
|
|
- Live E2E passed for demo compatibility:
|
|
- Start: `mvn spring-boot:run "-Dspring-boot.run.profiles=mvp-demo"`
|
|
- Run: `powershell -ExecutionPolicy Bypass -File mvp/demo/scripts/run-payment-timeout-demo.ps1`
|
|
- Result: `/api/chat`, `/api/diagnosis/{sessionId}/trace`, and `/api/feedback` completed successfully.
|
|
- Trace summary included `hasVerifierEvaluation=true`.
|
|
- Persisted Gatekeeper audit included `rule_set_version=gatekeeper-rules-v1`.
|
|
- Residual quality note: the live payment-timeout response remained `LOW_CONFID` because some model-produced evidence bindings still lacked explicit `source_invocation_id`; deterministic PASS/LOW_CONFID/REJECT claims are covered by fixture-backed eval.
|
|
|
|
## Archive Readiness
|
|
|
|
- OpenSpec tasks 1-4 completed.
|
|
- Verification is recorded in devflow acceptance artifacts.
|
|
- Remaining known risk: live LLM output is not deterministic and may still produce LOW_CONFID on the payment-timeout path; this is intentionally documented as demo compatibility, not a fixed PASS guarantee.
|