Files
ss-tools/specs/REVIEW-042-047-CLOSURE.md

18 KiB
Raw Blame History

Review Closure — 042047 Architectural Gaps (2026-08-07)

Purpose: Trace every review finding from the 042047 architecture review to a concrete resolution and the spec/file that changes. Prevents silent regression and keeps the audit trail. P0 items are binding; P1/P2 documented.

Reading: # = review finding (127 from the review). = resolution. [FILE] = file updated.


P0 — Binding fixes

#1. Scenario never reaches the Registry after creation

Problem: 042 has GET /scenarios but no POST /scenarios; 039 Save → 042 registration gap. Resolution: Add CreateScenario transaction to 042: input {validated_revision, draft_pack, owner, dashboard}, atomic ScenarioRegistryEntry + ScenarioRevision #1 + artifact materialization binding, output {scenario_id, revision_id + content_hash}. 039 Save calls it. Add task + route + spec story. [FILE] 042/contracts/openapi.yaml (POST /scenarios), 042/data-model.md (CreateScenario), 042/tasks.md (T0xx), 042/spec.md (US: Save→Register).

#2. ScenarioRevision ↔ runner.plan.json can diverge

Problem: r18 graph edited, runner.plan.json still r17 → run claims r18 but executes r17. Kills reproducibility. Resolution: RunnerPlan is a deterministic derivation from ScenarioRevision at run start, never a stored runtime source of truth. The run compares its derived revision/program/action-registry hashes to the selected revision before execution. [FILE] 044/data-model.md, 044/contracts/modules.md (RunnerPlan derivation), 042/data-model.md (revision carries runner_plan_hash).

#3. Wrong reuse of 036 ApprovalGate for human checkpoint / ScenarioRun

Problem: 036 gate contract is only repository_write | baseline_approval, owned by an AgentRun; ScenarioRun is separate; false_positive/inconclusive aren't 036 decisions. Resolution: Split into two domain concepts:

  • ActionApprovalGate — PROD execution approval, baseline approval, repository mutation. Reuses 036 gate mechanism, but generalized owner (see #4).
  • HumanCheckpoint — test observation disposition: confirm | false_positive | inconclusive. A separate entity with its own lifecycle, not a 036 gate. [FILE] 044/data-model.md, 044/contracts/modules.md, 044/spec.md (Clarifications), 044/contracts/openapi.yaml (human/decision schema).

#4. Execution artifacts still tied to AgentRun

Problem: 038 capture / VLM require agent_run_id; ScenarioRun (agent_run_id=null) can't supply it. Evidence/report/xlsx/screenshot shouldn't be DraftArtifact of an artificial AgentRun. Resolution: Introduce a generic artifact ownerArtifact { id, owner_type: agent_run|scenario_run|verification_run|load_run, owner_id, kind, sha256, content_ref, retention_class, ... }. ScenarioRun evidence uses owner_type=scenario_run. Add ScenarioRunArtifact projection. Evidence bridge accepts owner_type (036 evidence adapter generalized). [FILE] 044/data-model.md, 044/contracts/modules.md, 042/data-model.md (retention), 044/spec.md.

#5. Backend contract for 045 missing (history/result/compare/retry/SSE)

Problem: 045 UI promises RunHistory/RunComparison but 044 has no GET /scenarios/{id}/runs, GET /scenario-runs/{id}/result, GET /scenario-runs/compare, step retry, or an SSE event schema. Resolution: Add to 044 OpenAPI: GET /scenarios/{id}/runs, GET /scenario-runs/{id}/result, GET /scenario-runs/compare?a=&b=, POST /scenario-runs/{id}/steps/{logical_step_id}/retry, and an SSE event contract (id, sequence, event_type, run_id, logical_step_id?, attempt?, occurred_at, payload; Last-Event-ID replay, heartbeat, terminal close) — reuse the 036 AgentRunEvent pattern. [FILE] 044/contracts/openapi.yaml, 044/contracts/modules.md, 044/spec.md.

#6. Durable worker semantics (lease/heartbeat/idempotency/crash recovery)

Problem: crash-recovery claim needs at-least-once primitives, not a service loop. Resolution: Add worker runtime primitives: worker lease, heartbeat, run claim, step claim, lease expiration, idempotency key, recovery scheduler. Each executor declares idempotent? | retry-safe? | side-effect key? | external request id?. POST /scenario-runs requires Idempotency-Key to prevent double-run on double-click. [FILE] 044/data-model.md, 044/contracts/modules.md, 044/contracts/openapi.yaml, 044/tasks.md.

#7. 043 edit contract bypass (arbitrary draft)

Problem: save accepts draft: object → client can bypass the constrained editor. Resolution: Server-stored WorkingDraft. apply persists a WorkingDraft server-side and returns draft_id + digest. save(draft_id, digest) reloads it server-side, re-validates, canonicalizes, re-hashes, compares base revision. Client never returns the full graph. [FILE] 043/data-model.md, 043/contracts/modules.md, 043/contracts/openapi.yaml, 043/tasks.md.

#8. Stable logical step identity

Problem: an ordinal-derived step identity can shift on edit; breaks 045 compare + 047 flakiness across revisions. Resolution: logical_step_id = UUID (immutable) + step_position (mutable) + step_content_hash (mutable). Analytics key on logical_step_id. [FILE] 044/data-model.md, 042/data-model.md, 045/data-model.md, 047/data-model.md.

#9. Revalidation / Migration workflow (staleness remediation)

Problem: "Revalidate" button has no defined flow. Resolution: Scenario Migration workflow in 043: stale → revalidate against current dashboard → automatic mappings + manual conflicts → proposed r18 → diff → approve. Add spec story + tasks. [FILE] 043/spec.md, 043/data-model.md, 043/tasks.md, 042/spec.md.

#10. Global Run Operations Center + Automation Management UI

Problem: 045 covers Scenario→Runs only; 046 is backend-only. Resolution:

  • 045: add Global Run Operations Center (/dashboard-testing/runs) — all active/queued/waiting-human/failed/recent runs with filters, incl. "Waiting for me" for human checkpoints.
  • 046: add Automation Management UI — schedules/triggers/policies CRUD surface. [FILE] 045/spec.md, 045/data-model.md, 045/tasks.md, 046/spec.md, 046/contracts/ux/.

P1/P2 — Documented resolutions

#12. Run Configuration vs 044 start API mismatch

Resolution: 044 POST /scenario-runs gains release, baseline_set, execution_toggles, approval_ref (optional), matching 045 RunConfiguration. [FILE] 044/contracts/openapi.yaml.

#16. Result aggregation truth table

Resolution: Define step→assertion→scenario aggregation formally. A step outcome maps to a normalized contribution; scenario result derived with explicit rules (e.g., any failed → failed; skipped doesn't count against pass; blocked descendants counted as blocked, not failed). Add truth table. [FILE] 044/data-model.md.

#17. Execution toggles safety

Resolution: Toggles may only disable optional evidence enrichment (diagnostic screenshots, verbose logs, optional VLM commentary). Mandatory graph steps cannot be toggled off; otherwise run would false-PASS. [FILE] 045/spec.md, 044/contracts/openapi.yaml.

#18. (covered by #9) Revalidation = 043 migration workflow.

#20. Automation Management UI → 046 (see #10).

#21. API trigger operation

Resolution: Add POST /scenarios/{id}/trigger (external API run trigger) to 046, distinct from creating a ScenarioTriggerRule(trigger=api). [FILE] 046/contracts/openapi.yaml.

#22. Scheduler semantics

Resolution: Specify timezone, DST, misfire_grace_time, coalesce, max_instances, missed-execution policy (run-immediately | skip | queue), scheduler-restart handling. [FILE] 046/data-model.md, 046/spec.md.

#23. Retention vs analytics conflict

Resolution: Layered retention tiers (run metadata 180d, triage/audit 365d/policy, step metrics 90d, heavy artifacts 30d, screenshots 30d, raw VLM 7d) and an analytics minimum history window independent of retention. [FILE] 046/data-model.md, 047/data-model.md.

#24. 047 resource shape

Resolution: Change 047 endpoints to GET /scenarios/{scenario_id}/health|trends|recurring-failures (health/trends aggregate scenario history, not one run). [FILE] 047/contracts/openapi.yaml.

#25. Strict flakiness rules

Resolution: Define window + eligibility (same environment class, logical step, compatibility_family, baseline family) + flaky iff pass AND fail observed AND failure ratio within (X,Y) AND infra failures excluded. [FILE] 047/data-model.md.

#26. Immutable recurring fingerprint

Resolution: Fingerprint built from immutable raw evidence: logical_step_id + error_code + normalized_error_signature + assertion_kind + affected_ref. Triage classification is a separate group attribute, never part of fingerprint. [FILE] 047/data-model.md.

#27. Triage split

Resolution: RunResult = immutable truth; triage = { investigation_status: new|investigating|resolved, classification, resolution: fixed|accepted_risk|duplicate|wont_fix }. Run stays FAILED; triage is orthogonal. [FILE] 047/data-model.md, 047/contracts/modules.md.


Updated scores (target after closure)

Spec Concept Contractual closure
042 Registry 8.5 8.0
043 Editor 8.5 8.0
044 Execution 9.0 8.0
045 Run Monitor 9.0 8.0
046 Automation 8.0 7.5
047 Analytics 8.0 7.5

Cross-Spec Canonicalization Pass (2026-08-07, second pass)

Reconciled the stale 038 core with 042047. Bindings applied to 038 and all normative (spec/research/checklists/ux/prototype):

  • #1/#7 038 identity: scenario_id slug → scenario_key (semantic); scenario_id (UUID) + revision_id (UUID) assigned by 042 at Save; compiler emits content_hash only.
  • #2/#8/#11 Replaced revision_id + content_hash/parent_revision_id + content_hash/scenario_revision_id + scenario_content_hash with revision_id/content_hash/parent_revision_id across 042/043/044/045 (openapi, data-model, modules, spec, research, checklists, tasks). 043 unified fully.
  • #3/#4/#13 038 step schema: added logical_step_id (UUID, immutable) + step_key/position/step_content_hash; runtime VlmFinding/HumanDisposition moved to 044; VlmAnalysisSpec/ScreenshotCaptureSpec stay in 038.
  • #5/#6/#12 038 runtime capture/VLM/disposition endpoints marked deprecated→410 MOVED_TO_044; agent_run_id removed from compile/capture (provenance optional, source_type: agent_run|editor|migration|api). Runtime evidence = Artifact(owner_type=scenario_run), never authoring DraftPack.
  • #8 false_positive vocabulary unified; dismiss removed (037 dispositions split; 047 triage split investigation_status/classification/resolution).
  • #9/#10 038 validation.md PASS nullified (self-contradictory COMPLETE vs OPEN); refocused as compiler-layer PASS only; T057T059 moved to 044; tasks.md T046 rewritten.

Machine-Contract Reconciliation Gate (2026-08-09)

Closed the P0 "prose-fixed, contracts-stale" gap by validating the machine-readable source of truth:

  • OpenAPI YAML: fixed 3 syntax failures (038 line 472 unquoted :, 043 line 32 flow {...}, 046 line 23 & 51 Schedule[]/TriggerRule[]). All 036047 OpenAPI now parse.
  • 038 JSON Schema (dashboard-test-scenario.schema.json): migrated to canonical identity — scenario_idscenario_key, revision_id + content_hashcontent_hash; step idlogical_step_id+step_key+position+step_content_hash; vlm_analysisvlm_analysis_spec; affected_step_idsaffected_logical_step_ids; coverage.step_idslogical_step_ids; disposition dismissedfalse_positive.
  • 038 fixtures (fixtures/api/*.json, 6): migrated to canonical identity via transform script; fixed 66-char fingerprint; 6/6 validate against the JSON Schema (jsonschema).
  • 038 validation.md: regenerated cleanly (was self-contradictory PASS). Compiler-scope PASS; runtime gated by 044.
  • 039 prototype: scenario_idscenario_key in JSON display; 038 prototype revision_id + content_hashcontent_hash.
  • New tool reconcile_contracts.py: parses all OpenAPI/JSON, checks forbidden old-identity tokens in machine files (with compiled-output scoping for scenario_id), validates fixtures vs schema. Gate: PASS (0 findings) on 038047 and all.

Follow-up items listed here were resolved by the 2026-08-10/11 canonical contracts below; implementation must use those newer sections as normative source.

Status

  • Closure doc written
  • 042-047 edits applied (previous passes)
  • Machine-Contract Reconciliation Gate applied (038 schema+fixtures+validation, OpenAPI YAML fixes, reconcile_contracts.py) — PASS 0 findings

Contract Closure / Canonicalization Pass (2026-08-10)

The HOLD review has been incorporated as binding canonical contracts before implementation:

  • reconcile_contracts.py now performs OpenAPI path-template semantic validation and a narrow rejected-decision drift scan; all 036047 contracts pass.
  • 036 ActionApprovalGate is generic (owner_type, owner_id, scenario/load operations); the agent route is explicitly an adapter.
  • 044 creates durable ScenarioRun(pending_approval) plus a gate for PROD, pins ParameterBinding, TargetSnapshot, and ExecutionPrincipal provenance, defines CAS HumanCheckpoint, and limits browser recovery to deterministic checkpoint replay.
  • 038 contains immutable ParameterDefinition only; runtime values/statuses were removed from its JSON Schema and fixtures. Logical step UUIDs are minted once and carried through revisions; neither position nor ordinal affects identity.
  • 042 owns metadata versions separately from executable revisions, records clone provenance without cross-scenario revision parents, aggregates/deduplicates active staleness signals, and defines the object-level ACL intersection.
  • 043 separates metadata and executable edits, types parameter values as JSON validated against ParameterDefinition, and completes proposal/revalidation → WorkingDraft → save.
  • 045 uses the pre-run scenario configuration resource, exposes every executor state/filter, and separates mandatory checks from optional diagnostics.
  • 046 exposes schedule/trigger/policy CRUD schemas, derives coalescing from missed-execution policy, requires external-trigger idempotency, and delegates cross-run quota to 044's shared capacity manager.
  • 047 keys analytics on compatibility_family and logical_step_id, splits health dimensions, and uses alertable recurring-failure episodes after resolution.

Verification: backend/.venv/bin/python reconcile_contracts.py 036-047PASS (0 findings); git diff --checkPASS.

Final Execution-Semantics Closure (2026-08-10)

The second HOLD review is resolved with these binding decisions:

  1. 038 authoring validity permits unbound required ParameterDefinitions; 044 alone performs per-launch RunPreflight/ParameterBinding.
  2. 036 exposes generic ActionApprovalGate read/decision/consume routes and operation-specific ScenarioExecution approval requests.
  3. All entry points share the 044 PROD lifecycle: accepted run is queued or pending_approval, never an approval-only 403.
  4. 038 ActionRegistry is version-pinned; 044 BrowserExecutor dispatches only registered {tool, action} contracts.
  5. Mutation has a distinct immutable contract. MVP prohibits all PROD mutation and requires controlled test-data scope, cleanup and non-retry default elsewhere.
  6. Idempotent replay returns the prior run for the same key/request hash; different request reuse is 409.
  7. Metadata has an ETag PATCH route; executable draft save accepts confirmation only and derives its audit summary server-side.
  8. Revalidation includes conflict-resolution before proposal acceptance.
  9. 044 publishes typed result, comparison and SSE schemas; checkpoint outcome is selected by checkpoint type/policy.
  10. 046 separates dedup identity from capacity concurrency; 042 consumes (not derives) 047 health; 047 uses compatibility family and alertable FailureEpisodes.

reconcile_contracts.py now enforces 20 final-closure clauses in addition to parsability, schemas, semantic path parameters, fixtures, identity drift and rejected-decision patterns. The stale 041 task digest has also been corrected; future validation-table generation remains a separate tooling improvement.

Verification Program Reconciliation (2026-08-11)

The canonical system model is agent-authored, deterministically executed verification programs:

  • 038 now makes VerificationProgram required, content-hashed IR: navigation, evidence, bounded transforms, assertions and explicitly declared semantic evaluation.
  • Source-mart SqlEvidenceSpec is permitted only during authoring/edit/revalidation/investigation proposal, passes AST/policy/schema/preview validation and is executed at runtime unchanged through the Superset SQL Lab adapter with typed bindings and pinned security context.
  • TransformSpec is bounded DSL; ComparisonSpec is first-class; arbitrary code and runtime SQL/DSL rewrite remain forbidden.
  • 044 orchestration stays deterministic but supports bounded, versioned AgentEvaluationSpec; immutable AgentEvaluation evidence becomes StepOutcome only through DecisionPolicy.
  • 039 previews program structure, evidence sources and SQL/DSL/assertion/evaluation diffs; ChangeRequestContext is explicit and required for compilation.
  • 045 renders typed events/results, including DecisionPolicy outcomes and AgentEvaluation summaries, without prose parsing.
  • 047 distinguishes deterministic failure from model disagreement, low confidence and model instability; only deterministic outcomes feed deterministic flakiness.

Generated gate command: backend/.venv/bin/python reconcile_contracts.py 036-047 validates YAML/JSON, OpenAPI paths, JSON fixtures, legacy drift, Verification Program schema/action registry and runtime mutation boundary. Current result: PASS (0 findings).