Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@ summary: Chronological history of repository and skill changes.

# Changelog

## 2026-07-29 — Designed the review-fix-loop skill, migrated implement-ticket and babysit-pr to consume the final review-result contract, rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect, added connector-outcome curation and promotion tooling, added a skill-root override for mechanism ablation runs, ran the preregistered v2 ablation and integration closeout, and confirmed the session-continuation-summary verification-only regression with an independent rerun
## 2026-07-29 — Sourced two harder discriminating cases for the traversal and verification-sufficiency passes, designed the review-fix-loop skill, migrated implement-ticket and babysit-pr to consume the final review-result contract, rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect, added connector-outcome curation and promotion tooling, added a skill-root override for mechanism ablation runs, ran the preregistered v2 ablation and integration closeout, and confirmed the session-continuation-summary verification-only regression with an independent rerun

- docs(review-suite): validate the two new discriminating cases with-pass and
without-pass, fixing a construction defect found in the traversal case along
the way, and report the traversal pass discriminates while the
verification-sufficiency pass still does not (#89)
- feat(review-suite): source two harder discriminating
`s1-correctness-orchestrator` cases for the traversal and
verification-sufficiency passes and preregister their validation ceiling (#89)
(`bfec2910a81422df365ddc3ba4c70672a9ebe269`)
- docs: design the review-fix-loop skill
(`06538e5c097ff8e6ef15b12d5fbf61b3d959abf7`)
- docs(review-suite): add a confirming rerun of the session-continuation-summary
verification-only regression (#57 follow-up)
(`cd8efa444018d036a5749a1955e1f34ebe06b51f`)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"corpus_version": "0.1-s1-populated",
"corpus_version": "0.2-s1-populated",
"protocol_version": "1.0",
"grader_version": "1.1",
"target_skill": "review-code-change",
Expand All @@ -13,9 +13,11 @@
"ground_truth": "human-review",
"scored": true,
"grading_is_signal": true,
"purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss."
"purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it."
},
"cases": [
"artifact-promotion-environment-shortcut",
"audit-log-flush-keyword-probe",
"dependency-hint-parser-coverage",
"dependency-strictness-propagation",
"optional-tool-probe",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"expectation_version": "1.0",
"case_id": "artifact-promotion-environment-shortcut",
"packet_valid": true,
"calibrated": false,
"expected_verdict": "changes_required",
"material_root_causes": [
{
"id": "rc.rollout-path-keeps-marker-only-promotion-check",
"requirement": "Every path that decides whether an artifact counts as promoted applies the same mandatory-verification rule once verification is required for that artifact.",
"trigger": "An artifact requires mandatory verification, carries a promotion marker from the release tool, but has no ledger-confirmed proof, and enters the staging environment-selection step.",
"surface": "_pick_rollout_defaults",
"consequence": "`resolve_rollout_environment` reaches `artifact_promotable` through `_pick_rollout_defaults` without ever passing `external_proof`, so the environment-selection worker still treats the artifact as promoted for staging-defaults purposes even though `decide_promotion` would hold its release. The diff only hardens the gate's own direct call; the two-hop rollout path keeps deciding the same question permissively, and the change contract's non-goal about rollout defaults reads as license to leave it alone rather than as a sign it was missed.",
"severity": "blocking",
"equivalent_formulations": [
"_pick_rollout_defaults still calls artifact_promotable without external_proof",
"the rollout path was not updated to require ledger proof",
"resolve_rollout_environment can still treat an unverified artifact as promoted",
"a marker-only artifact can still get promoted-tier rollout defaults",
"the verification requirement is applied inconsistently between the gate and the rollout path"
]
}
],
"accepted_non_findings": [
{
"id": "anf.optional-proof-parameter-shape",
"description": "A reviewer may prefer a distinct function over an optional `external_proof` parameter. Reasonable as design commentary; the packet's preserved-behaviour list keeps the permissive result reachable when verification is not required, so an optional parameter satisfies the contract.",
"equivalent_formulations": [
"prefer a separate function to an optional proof parameter",
"external_proof as an optional keyword argument is a design smell"
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"expectation_version": "1.0",
"case_id": "audit-log-flush-keyword-probe",
"packet_valid": true,
"calibrated": false,
"expected_verdict": "changes_required",
"material_root_causes": [
{
"id": "rc.retry-reuses-stale-optimistic-concurrency-script",
"requirement": "A retried flush re-derives its script from a fresh read of the entry's current version before resending.",
"trigger": "The first flush attempt reports a transient session error, and a concurrent flush advances the entry's stored version before the retry runs.",
"surface": "flush_entry",
"consequence": "The retry resends the exact same already-rendered script, whose `WHERE version = ...` clause now names a version older than the row's current one, so the session reports the script as syntactically successful while it matches zero rows and nothing is actually persisted on retry; `flush_entry` still records the entry as flushed. The added `_FakeSession` treats any script containing `COMMIT` as a persisted success and never models whether the version clause would actually match a row, so it exercises the retry path in name only and cannot tell a real persisted retry from this silent no-op.",
"severity": "blocking",
"equivalent_formulations": [
"the retry resends the same stale script instead of re-rendering it",
"the version used on retry is not refreshed from a fresh read",
"a concurrent flush between attempts can make the retry a silent no-op",
"the fake session only checks for COMMIT in the script text, not whether it actually persisted",
"the added test cannot distinguish a real persisted retry from a no-op retry"
]
}
],
"accepted_non_findings": [
{
"id": "anf.template-rendered-sql",
"description": "Building the script from a string template rather than a parameterized query API is unusual, but it is how this session boundary's script interface is used everywhere else in this module; not a defect here.",
"equivalent_formulations": [
"string templating is used to build the script",
"the script is built via string formatting instead of bound parameters"
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"provenance_version": "1.0",
"case_id": "artifact-promotion-environment-shortcut",
"origin": "minimized_reproduction",
"retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 315, review comment 2866874599, authored by the repository owner and accepted in the reply (comment 2866898151, \"Good catch; this is now aligned\") naming the tests added to cover it. Public source, owner-authored review, no third-party or customer material.",
"sanitization": "Rewritten from scratch against a fictional artifact-release system, deliberately reshaped to be less salient than this stratum's existing `dependency-strictness-propagation` case: the untouched sibling path is reached two calls deep (`resolve_rollout_environment` -> `_pick_rollout_defaults` -> `artifact_promotable`) rather than by one direct call, and it lives in a module (`rollout/targeting.py`) whose name and docstrings describe staging-environment defaults, not dependency or promotion checking. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.",
"adjudication": {
"first": "shaug/atelier PR 315 review comment 2866874599, accepted in reply 2866898151 (tests named: test_changeset_base_branch_uses_same_dependency_integration_signal_as_pr_gate, test_attempt_create_pr_passes_repo_slug_to_base_resolver).",
"second": "oracle"
},
"recorded_at": "2026-07-29",
"notes": "Sourced for #89 as a harder discriminating sibling to `dependency-strictness-propagation`, per #57's ablation matrix finding that the existing traversal-pass target case resolves identically whether or not the traversal pass is enabled. Multi-file, multi-hop case: the defect is only visible by reading a caller the diff does not touch and tracing one further call through a helper. Second adjudication is available by oracle."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"provenance_version": "1.0",
"case_id": "audit-log-flush-keyword-probe",
"origin": "minimized_reproduction",
"retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 755, review comment 3031027866, authored by the repository owner and accepted in the reply (comment 3031075465, describing the tightened regression) naming the concrete fix. Public source, owner-authored review, no third-party or customer material.",
"sanitization": "Rewritten from scratch against a fictional audit-log flush module. Retains only the failure shape: a retry path resends a stale precondition, and the accompanying fake test session grants success on a superficial keyword match in the request text rather than modelling whether the request actually persisted - the exact shape of `stale-claim-release-guard`'s test-insufficiency finding but disguised behind a mocked session boundary rather than an initial-state choice, so it is less visually obvious on a first read. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.",
"adjudication": {
"first": "shaug/atelier PR 755 review comment 3031027866, accepted in reply 3031075465 (regression tightened around the session boundary; a malformed-helper-evidence failure case added).",
"second": "oracle"
},
"recorded_at": "2026-07-29",
"notes": "Sourced for #89 as a harder discriminating sibling to `stale-claim-release-guard`, per #57's ablation matrix finding that the existing verification-sufficiency target case resolves identically whether or not the verification-sufficiency pass is enabled. The insufficiency is carried by a fake session's keyword-only success check rather than by the test's initial state, which is the harder-to-notice shape #89 asks for. Second adjudication is available by oracle."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
{
"schema_version": "1.0",
"repository": {
"identity": "example/releases",
"base_branch": "main"
},
"candidate": {
"head_sha": "57a56b3cd4d327512251eb72c62d72bdb8d606f9",
"comparison_base_sha": "59d8eb504311bc773d02d2d1975b707672441318",
"diff": {
"format": "unified_diff",
"complete": true,
"content": "diff --git a/promotion/gate.py b/promotion/gate.py\nindex 2cb8a07..01d515d 100644\n--- a/promotion/gate.py\n+++ b/promotion/gate.py\n@@ -1,8 +1,15 @@\n from promotion.model import Artifact\n \n \n-def artifact_promotable(artifact):\n- \"\"\"Return whether this artifact may be treated as promoted.\"\"\"\n+def artifact_promotable(artifact, *, external_proof=None):\n+ \"\"\"Return whether this artifact may be treated as promoted.\n+\n+ With `external_proof`, only a ledger-confirmed record counts. The\n+ promotion marker alone is written by the release tool and can be present\n+ on an artifact whose release announcement was never confirmed.\n+ \"\"\"\n+ if external_proof is not None:\n+ return bool(external_proof.verified)\n return bool(artifact.promotion_marker or artifact.external_ref)\n \n \n@@ -12,5 +19,10 @@ def hold_release(record):\n \n def decide_promotion(artifact, ledger):\n for record in [artifact]:\n- if not artifact_promotable(record):\n+ proof = (\n+ ledger.lookup(record.external_ref)\n+ if record.verification_required\n+ else None\n+ )\n+ if not artifact_promotable(record, external_proof=proof):\n hold_release(record)\ndiff --git a/tests/test_promotion_gate.py b/tests/test_promotion_gate.py\nindex 16ed52a..985aa13 100644\n--- a/tests/test_promotion_gate.py\n+++ b/tests/test_promotion_gate.py\n@@ -8,3 +8,16 @@ def test_artifact_promotable_accepts_a_promotion_marker():\n \n def test_artifact_promotable_accepts_an_external_ref():\n assert artifact_promotable(Artifact(external_ref=\"rel-9\"))\n+\n+\n+def test_artifact_promotable_rejects_a_marker_only_artifact_under_ledger_proof():\n+ artifact = Artifact(promotion_marker=True, external_ref=None)\n+ assert not artifact_promotable(artifact, external_proof=UnverifiedProof())\n+\n+\n+def test_decide_promotion_holds_a_verification_required_artifact_without_ledger_proof():\n+ artifact = Artifact(\n+ verification_required=True, promotion_marker=True, external_ref=None\n+ )\n+ decide_promotion(artifact, ledger=EmptyLedger())\n+ assert artifact.held\n"
}
},
"change_contract": {
"goal": "Stop treating an artifact as promoted on the strength of its promotion marker alone once that artifact requires mandatory verification, because the marker is written by the release tool and can be present before the ledger confirms the release actually landed.",
"acceptance_criteria": [
"An artifact that requires mandatory verification is only treated as promoted when the ledger confirms proof.",
"An artifact that does not require mandatory verification keeps today's marker/external-ref check."
],
"non_goals": [
"Change how the ledger records proof.",
"Change staging/rollout default selection."
],
"preserved_behaviors": [
"`artifact_promotable` keeps its permissive result when verification is not required.",
"`decide_promotion` keeps its existing signature."
]
},
"sources": {
"repository_instructions": [
{
"label": "Verification rules",
"location": "AGENTS.md",
"summary": "Once an artifact requires mandatory verification, only ledger-confirmed proof may satisfy a promotion decision; the promotion marker alone is not sufficient once that requirement applies."
}
],
"named_documents": [
{
"label": "Promotion proof contract",
"location": "docs/promotion.md"
},
{
"label": "Artifact model",
"location": "promotion/model.py"
}
],
"nearby_patterns": [
{
"label": "Ledger client",
"location": "promotion/ledger.py"
},
{
"label": "Rollout defaults",
"location": "rollout/targeting.py"
},
{
"label": "Promotion gate tests",
"location": "tests/test_promotion_gate.py"
}
]
},
"validation": [
{
"name": "promotion gate tests",
"command": "pytest tests/test_promotion_gate.py",
"scope": "focused",
"status": "passed",
"result": "7 passed"
},
{
"name": "full tests",
"command": "pytest",
"scope": "full",
"status": "passed",
"result": "146 passed"
}
],
"context": {
"data": [
"`promotion/gate.py` and `rollout/targeting.py` are the only two modules that call `artifact_promotable`. This diff does not touch `rollout/targeting.py`, whose relevant part is:\n\n def _pick_rollout_defaults(artifact):\n \"\"\"Choose default staging parameters for an artifact entering the environment.\"\"\"\n if not artifact_promotable(artifact):\n return STAGED_DEFAULTS\n return PROMOTED_DEFAULTS\n\n def resolve_rollout_environment(artifact):\n \"\"\"Return the environment config used when a worker enters an artifact into staging.\"\"\"\n defaults = _pick_rollout_defaults(artifact)\n return build_environment(defaults, artifact)",
"`resolve_rollout_environment` is called by the environment-selection worker whenever an artifact enters staging.",
"An artifact with `verification_required=True` can still carry a `promotion_marker` from the release tool whenever the tool ran before the ledger recorded confirmation; this happens whenever a release is announced before its ledger entry settles.",
"`ledger.lookup(external_ref)` always returns a proof object, never `None`: `proof.verified` is `False` whenever there is no external ref or the ledger holds no confirming record for one. A promotion decision never treats a `None` lookup result as \"verification not required\" - the only way `external_proof` is `None` in `artifact_promotable` is when the caller does not pass it at all (verification not required for that artifact)."
],
"operational": [
"The environment-selection worker runs once per staging entry and does not re-evaluate later; whatever defaults it assigns at entry persist for the artifact's whole staging window."
]
}
}
Loading
Loading