[K4] Bind candidate acceptance to exact verification evidence #21

Merged
adrian.tkacz merged 1 commit from issue-7-evidence-verification into develop 2026-09-26 11:26:27 +00:00
Member

Closes neither issue before integration. Implements #7 against develop.

  • Strictly parse untrusted candidate output bindings and bind them to a collected artifact manifest, finished Attempt, approved spec, pinned policy and exact repository/base/head.
  • Require fresh externally produced deterministic and cold-review evidence before creating trusted satisfaction; revalidate at admission. Track findings, finite remediation/progress/oscillation and pause versus HumanGate.
  • Add bounded ContextPackage and local outcome export with identity attribution, unknown usage and correction lineage.
  • Synthetic two-node lifecycle and C/D/E/F negative-path tests, stale subjects, missing/tampered artifacts, changed head/policy, and duplicate receipts.

Validation: uv run --locked pytest -q tests/test_verification.py (10 passed); make check (179 passed, lint/type/import/secret/diff checks); make package-check (isolated wheel install passed). No live service or worker execution claimed. Issue remains open pending owner integration.

Closes neither issue before integration. Implements #7 against `develop`. - Strictly parse untrusted candidate output bindings and bind them to a collected artifact manifest, finished Attempt, approved spec, pinned policy and exact repository/base/head. - Require fresh externally produced deterministic and cold-review evidence before creating trusted satisfaction; revalidate at admission. Track findings, finite remediation/progress/oscillation and pause versus HumanGate. - Add bounded ContextPackage and local outcome export with identity attribution, unknown usage and correction lineage. - Synthetic two-node lifecycle and C/D/E/F negative-path tests, stale subjects, missing/tampered artifacts, changed head/policy, and duplicate receipts. Validation: `uv run --locked pytest -q tests/test_verification.py` (10 passed); `make check` (179 passed, lint/type/import/secret/diff checks); `make package-check` (isolated wheel install passed). No live service or worker execution claimed. Issue remains open pending owner integration.
feat: bind candidate acceptance to exact verification evidence
All checks were successful
ci / check (pull_request) Successful in 19s
06f14f217d
kilo left a comment

PR Review: [K4] Bind candidate acceptance to exact verification evidence (#21)

Repository: Creatidy/creatidy-kernel
Branch: issue-7-evidence-verification -> develop
Frozen head: 06f14f217d
Linked issue: #7 ([K4] Evidence-based verification, acceptance, remediation, and outcomes)

Independent read-only review in a fresh reviewer context. Local checkout verified clean at the exact frozen head; base develop @ e41f692 equals the merge base (no drift). 5 files, +1221, all additive (core/verification.py, core/context.py, core/outcomes.py, tests/test_verification.py, tests/test_context_outcomes.py). No prior reviews or comments existed.

Platform identity limitation: this review is submitted by the Forgejo account kilo, which is also the PR author. A formal APPROVED submission was attempted and rejected by the platform ("approve your own pull is not allowed"), so this review persists as COMMENT. The substantive independent verdict below is APPROVE and must not be inferred from the persisted COMMENT state.

Summary

Implements the K4 verification/acceptance seam: strict untrusted-candidate parsing bound to a collected artifact manifest, finished Attempt, current approved spec, pinned policy and exact repository/base/head; fresh exact-subject deterministic and cold-review evidence required before a TrustedSatisfaction is created and revalidated at admission; bounded remediation with progress/oscillation/pause/HumanGate distinction; bounded ContextPackage manifests; and a pure local outcome export with identity attribution, unknown-usage preservation and superseding correction lineage.

Intent / Acceptance Evidence

Issue #7 acceptance criterion Evidence at frozen head
Schema-valid candidates cannot be accepted without exact subjects and required fresh evidence verify_candidate requires per-check item.subject == subject (candidate digest + attempt digest + spec digest + policy digest + repo/base/head) and observed_at <= now <= fresh_until; missing/failing/stale/mismatched evidence yields findings and RejectedResult (tests/test_verification.py::test_schema_valid_proposal_is_not_accepted_without_exact_fresh_evidence, incl. worker-produced and subject-override variants). Claimed verdict fields are never parsed (parse_candidate rejects any extra field such as pass).
Bounded remediation when authorized; progress recognized, oscillation diagnosed, only genuine authority need creates HumanGate assess_remediation + test_f_new_blockers_progress_oscillation_pause_and_authority_gate: retry within budget, PAUSE on exhausted/unauthorized budget, HUMAN_GATE only for owner_decision_required; fingerprints tracked across changed subjects without relabeling.
C/D/E/F explicit tests C: test_c_malformed_extra_duplicate_candidate_data (5 parametrized cases: non-JSON, non-object, duplicate top-level key, extra field, duplicate output key). D/E: test_d_e_transport_success_is_not_domain_or_review_acceptance models domain rejection (failing review) and unaccepted wait (missing review) with no satisfaction and dependent unit still PENDING. F: test above.
Two-node synthetic flow; node-one verified output becomes node-two input; stale subjects rejected, old findings preserved test_two_node_acceptance_and_stale_subject_preserves_old_evidence: acceptance yields predecessor InputBinding with satisfaction id and output provenance; second node accepted -> COMPLETED; re-verifying the stale first candidate raises StaleRevision; prior aggregate state unchanged; finding history preserved across remediation subjects.
Context/outcome manifests pin revisions/digests; unknown resource/usage preserved ContextPackage.digest covers pinned sources/revisions/digests/policies/compiler/budgets and matches binds attempt + spec + context_reference; UsageObservation preserves quantity=None, Provenance distinctions, quota freshness; corrections require exact superseding lineage and duplicate receipts fail closed (tests/test_context_outcomes.py).
Verification outside worker authority; no M5-B/workflow language Verifier must be a trusted_satisfaction_issuers member and differ from the attempt actor; every evidence producer must differ from the attempt actor; independent review additionally requires a distinct reviewer session and producer != verifier. `rg "M5

Architecture / Information Flow

Dependencies point inward only: the three new core modules import core.domain, core.execution, core.resources and stdlib; lint-imports reports contracts kept (1 kept, 0 broken). Parsing, decision and admission are separated from evidence production via the CheckProducer protocol, matching the "Verification" row of docs/architecture/contracts.md and ADR 0005 (schema eligibility != acceptance; missing evidence is not a pass; cold reviewer seam; no implementer-context reuse). admit_accepted replays stored evidence against the recomputed current subject, so changed head/base/policy/spec or tampered/empty evidence fails closed with StaleRevision before SatisfyWorkUnit is applied — consistent with the acceptance/merge-race contract. ContextPackage/outcome export match ADR 0006 (bounded manifest, no vector store, unknown-not-zero usage, superseding corrections, remediation cost linked to accepted outcome via cost_attempt_ids). All fixtures are synthetic; no private, live or cloud services are touched.

Blocking Issues

None.

Non-blocking Suggestions

  1. Finding is the only new public record without __post_init__ invariant validation (its siblings all validate). It is constructed internally from validated inputs today; adding the same guard would keep the value style uniform.
  2. assess_remediation reports progress=True for a single-rejection history (no baseline). Reasonable policy choice, but worth one docstring sentence so callers do not read it as measured progress.
  3. The reviewer_session != spec.attempt_id guard compares a session identity to an attempt ID; it is a weak degenerate-identity check. Real separation currently comes from producer != attempt.actor_id and producer != verifier_id; consider strengthening once reviewer-session identity semantics are defined.
  4. No focused test hands admit_accepted an AcceptedResult whose satisfaction was tampered (e.g. swapped output bindings); the recompute-and-compare (checked != result) path covers it indirectly via the evidence=() test only.

Questions for the Author

None blocking. D/E are exercised at the verification seam (missing/failing review evidence) rather than through a transport-status path; transport-vs-domain separation conformance belongs to the Runtime port cases in docs/architecture/contracts.md. Confirm that reading, or state the intended split.

Security / Privacy Notes

parse_candidate uses an object_pairs_hook that rejects duplicate keys, requires an exact field set, string-only outputs, and never interprets claimed verdicts. Worker/verifier/reviewer separation is enforced in code, not prose. Candidate output paths must exist in the trusted collected manifest with unique paths and a workspace key pinned to the Attempt. No secrets, credentials or network access are introduced; the secret scan is clean.

Test / Documentation Gaps

Minor: the tampered-satisfaction admission test above; docs/architecture/successor.md K4 row still reads as plan text now that the kernel-side implementation exists (an implemented-status annotation would keep the plan document aligned, non-urgent). Validation claims in the PR body were independently re-run at the frozen head and all reproduced (below).

Follow-Up Candidates

  • Runtime-port transport/domain rejection conformance cases (already planned as port obligations in docs/architecture/contracts.md; owner triage only).

Validation (re-run by reviewer at frozen head 06f14f2)

  • uv run --locked pytest -q tests/test_verification.py tests/test_context_outcomes.py — 15 passed.
  • make check — uv lock ok; ruff format/check ok; basedpyright 0 errors; lint-imports contracts kept; 179 passed; secret scan clean; git diff --check clean.
  • make package-check — wheel metadata/license, sdist rebuild and isolated install passed.

Overall Recommendation

APPROVE — no blocking findings against the frozen head; issue #7 acceptance criteria are met within the declared K4 scope and constraints. Per the review contract, this substantive verdict stands independently of the persisted Forgejo COMMENT state caused by the shared PR-author identity noted above.

## PR Review: [K4] Bind candidate acceptance to exact verification evidence (#21) **Repository:** Creatidy/creatidy-kernel **Branch:** issue-7-evidence-verification -> develop **Frozen head:** 06f14f217d0975ff0a5bfc3d5d8c14ae84f331d1 **Linked issue:** #7 ([K4] Evidence-based verification, acceptance, remediation, and outcomes) Independent read-only review in a fresh reviewer context. Local checkout verified clean at the exact frozen head; base `develop` @ e41f692 equals the merge base (no drift). 5 files, +1221, all additive (`core/verification.py`, `core/context.py`, `core/outcomes.py`, `tests/test_verification.py`, `tests/test_context_outcomes.py`). No prior reviews or comments existed. **Platform identity limitation:** this review is submitted by the Forgejo account `kilo`, which is also the PR author. A formal APPROVED submission was attempted and rejected by the platform ("approve your own pull is not allowed"), so this review persists as COMMENT. The substantive independent verdict below is APPROVE and must not be inferred from the persisted COMMENT state. ### Summary Implements the K4 verification/acceptance seam: strict untrusted-candidate parsing bound to a collected artifact manifest, finished Attempt, current approved spec, pinned policy and exact repository/base/head; fresh exact-subject deterministic and cold-review evidence required before a `TrustedSatisfaction` is created and revalidated at admission; bounded remediation with progress/oscillation/pause/HumanGate distinction; bounded `ContextPackage` manifests; and a pure local outcome export with identity attribution, unknown-usage preservation and superseding correction lineage. ### Intent / Acceptance Evidence | Issue #7 acceptance criterion | Evidence at frozen head | | --- | --- | | Schema-valid candidates cannot be accepted without exact subjects and required fresh evidence | `verify_candidate` requires per-check `item.subject == subject` (candidate digest + attempt digest + spec digest + policy digest + repo/base/head) and `observed_at <= now <= fresh_until`; missing/failing/stale/mismatched evidence yields findings and `RejectedResult` (`tests/test_verification.py::test_schema_valid_proposal_is_not_accepted_without_exact_fresh_evidence`, incl. worker-produced and subject-override variants). Claimed verdict fields are never parsed (`parse_candidate` rejects any extra field such as `pass`). | | Bounded remediation when authorized; progress recognized, oscillation diagnosed, only genuine authority need creates HumanGate | `assess_remediation` + `test_f_new_blockers_progress_oscillation_pause_and_authority_gate`: retry within budget, PAUSE on exhausted/unauthorized budget, HUMAN_GATE only for `owner_decision_required`; fingerprints tracked across changed subjects without relabeling. | | C/D/E/F explicit tests | C: `test_c_malformed_extra_duplicate_candidate_data` (5 parametrized cases: non-JSON, non-object, duplicate top-level key, extra field, duplicate output key). D/E: `test_d_e_transport_success_is_not_domain_or_review_acceptance` models domain rejection (failing review) and unaccepted wait (missing review) with no satisfaction and dependent unit still PENDING. F: test above. | | Two-node synthetic flow; node-one verified output becomes node-two input; stale subjects rejected, old findings preserved | `test_two_node_acceptance_and_stale_subject_preserves_old_evidence`: acceptance yields predecessor `InputBinding` with satisfaction id and output provenance; second node accepted -> `COMPLETED`; re-verifying the stale first candidate raises `StaleRevision`; prior aggregate state unchanged; finding history preserved across remediation subjects. | | Context/outcome manifests pin revisions/digests; unknown resource/usage preserved | `ContextPackage.digest` covers pinned sources/revisions/digests/policies/compiler/budgets and `matches` binds attempt + spec + `context_reference`; `UsageObservation` preserves `quantity=None`, `Provenance` distinctions, quota freshness; corrections require exact superseding lineage and duplicate receipts fail closed (`tests/test_context_outcomes.py`). | | Verification outside worker authority; no M5-B/workflow language | Verifier must be a `trusted_satisfaction_issuers` member and differ from the attempt actor; every evidence producer must differ from the attempt actor; independent review additionally requires a distinct reviewer session and producer != verifier. `rg "M5|Prefect|workflow|needs_human"` over all new files: no matches. | ### Architecture / Information Flow Dependencies point inward only: the three new core modules import `core.domain`, `core.execution`, `core.resources` and stdlib; `lint-imports` reports contracts kept (1 kept, 0 broken). Parsing, decision and admission are separated from evidence production via the `CheckProducer` protocol, matching the "Verification" row of `docs/architecture/contracts.md` and ADR 0005 (schema eligibility != acceptance; missing evidence is not a pass; cold reviewer seam; no implementer-context reuse). `admit_accepted` replays stored evidence against the recomputed current subject, so changed head/base/policy/spec or tampered/empty evidence fails closed with `StaleRevision` before `SatisfyWorkUnit` is applied — consistent with the acceptance/merge-race contract. `ContextPackage`/outcome export match ADR 0006 (bounded manifest, no vector store, unknown-not-zero usage, superseding corrections, remediation cost linked to accepted outcome via `cost_attempt_ids`). All fixtures are synthetic; no private, live or cloud services are touched. ### Blocking Issues None. ### Non-blocking Suggestions 1. `Finding` is the only new public record without `__post_init__` invariant validation (its siblings all validate). It is constructed internally from validated inputs today; adding the same guard would keep the value style uniform. 2. `assess_remediation` reports `progress=True` for a single-rejection history (no baseline). Reasonable policy choice, but worth one docstring sentence so callers do not read it as measured progress. 3. The `reviewer_session != spec.attempt_id` guard compares a session identity to an attempt ID; it is a weak degenerate-identity check. Real separation currently comes from `producer != attempt.actor_id` and `producer != verifier_id`; consider strengthening once reviewer-session identity semantics are defined. 4. No focused test hands `admit_accepted` an `AcceptedResult` whose `satisfaction` was tampered (e.g. swapped output bindings); the recompute-and-compare (`checked != result`) path covers it indirectly via the `evidence=()` test only. ### Questions for the Author None blocking. D/E are exercised at the verification seam (missing/failing review evidence) rather than through a transport-status path; transport-vs-domain separation conformance belongs to the Runtime port cases in `docs/architecture/contracts.md`. Confirm that reading, or state the intended split. ### Security / Privacy Notes `parse_candidate` uses an `object_pairs_hook` that rejects duplicate keys, requires an exact field set, string-only outputs, and never interprets claimed verdicts. Worker/verifier/reviewer separation is enforced in code, not prose. Candidate output paths must exist in the trusted collected manifest with unique paths and a workspace key pinned to the Attempt. No secrets, credentials or network access are introduced; the secret scan is clean. ### Test / Documentation Gaps Minor: the tampered-satisfaction admission test above; `docs/architecture/successor.md` K4 row still reads as plan text now that the kernel-side implementation exists (an implemented-status annotation would keep the plan document aligned, non-urgent). Validation claims in the PR body were independently re-run at the frozen head and all reproduced (below). ### Follow-Up Candidates - Runtime-port transport/domain rejection conformance cases (already planned as port obligations in `docs/architecture/contracts.md`; owner triage only). ### Validation (re-run by reviewer at frozen head 06f14f2) - `uv run --locked pytest -q tests/test_verification.py tests/test_context_outcomes.py` — 15 passed. - `make check` — uv lock ok; ruff format/check ok; basedpyright 0 errors; lint-imports contracts kept; 179 passed; secret scan clean; `git diff --check` clean. - `make package-check` — wheel metadata/license, sdist rebuild and isolated install passed. ### Overall Recommendation APPROVE — no blocking findings against the frozen head; issue #7 acceptance criteria are met within the declared K4 scope and constraints. Per the review contract, this substantive verdict stands independently of the persisted Forgejo COMMENT state caused by the shared PR-author identity noted above.
adrian.tkacz deleted branch issue-7-evidence-verification 2026-09-26 11:26:27 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Creatidy/creatidy-kernel!21
No description provided.