feat(research): execute answerability proof surfaces - #7102
Conversation
github-actions
Bot
added
the
review-bot-auto
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Contract Check SummaryOverall Status: 🟢 PASSED
ℹ️ Info
This check is mechanized. Please resolve any blockers to pass CI. |
|
Exact-head review handoff for draft PR #7102 Reviewed head: Implementation verification:
Findings disposition:
Safety boundary:
|
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head refresh review
- Reviewed draft PR #7102 at head
37f9c59dddefa119f507c5ce94ae5e9a1f303875against base2cce5e2f0916524916a4592062e47b1b7b1dc433. - Scope is eight files: the typed answerability contract, shell-free proof collector, canonical manifest runner, example/fixture, docs, and focused tests.
- Required proof statuses are replaced from observed collectors before
--require-answerable; missing or required unavailable proof cannot be promoted toanswerable, while the absent generic result-packet validator remains explicitly unavailable. - Parent proof: 41 focused contract/preflight tests passed; Ruff check/format, canonical diagnostic CLI behavior, and
git diff --checkpassed. - Final readiness: follow-up
ok, checklist errors none; domain status is explicitlypending_domain_approvalbecause this changes launch-gate/evidence-validity admission behavior. - The PR remains draft. No campaign, compute, evidence admission, publication, or merge was performed or inferred.
No blocking implementation findings at this exact head. Domain review and draft disposition remain required.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head follow-up review for PR #7102
Reviewed commit: 562ea6804c33f572da2a3993031a5949590c60e0
Base: 2cce5e2f0916524916a4592062e47b1b7b1dc433 (origin/main)
The follow-up hardening is technically sound for the intended launch-gate contract:
- manifest success values now reject the complete caveated/non-executed status family:
fallback,degraded,diagnostic_only,unavailable,not_run,failed, andblocked, even when a caller supplies a narrower custom fail-closed list; - optional answerability proofs surface every non-
passedstatus as a warning, includingfailedandnot_run, so incomplete optional evidence remains visible; - typed
validation.answerability_proofargv adapters remain shell-free, while legacyvalidation.commandscompatibility behavior is explicitly outside that claim; - focused proof covers the production subprocess path and optional-status reporting: 51 tests passed, with Ruff, formatting, diff, and final advisory readiness passing.
The changed-line coverage warning is non-blocking under repository policy (88.7%, 47/53 executable lines). Strict readiness correctly remains blocked by pending_domain_approval; the PR remains draft and no campaign execution, evidence admission, publication, or merge is claimed.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
gate-verdict: accepted @ cba55a8
merge-ready: no
pr-metadata: reconciled @ 5a7da08c8cad4ed92113a7265a81fe896d284c80f73e7500bf957f21df401749
Exact-head implementation review accepted for the draft #7090 canonical answerability proof
composition. The refreshed head invokes typed producer, preregistration, evidence-contract,
analysis, and artifact checks through existing public owners, records absent generic result-packet
support as unavailable, replaces declarative statuses before --require-answerable, and rejects
incomplete artifact checksum declarations. Current proof is 55 focused tests, Ruff/format, docs
proof and evidence integrity, diff checks, and a diagnostic-only CLI smoke with no campaign.
This changes launch-gate and evidence-validity semantics, so Domain-Aware Approval and draft
disposition remain required. No merge-ready label, compute, campaign, evidence admission,
publication, or scientific result is authorized by this review.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
gate-verdict: accepted @ 75dbea5
merge-ready: no
pr-metadata: reconciled @ 6ed23439d3535aa6274c33e9cd39844bbb7856d69bf8a6dbee06eea08d9fa442
Exact-head implementation review accepted for the draft #7090 canonical answerability proof
composition. The refreshed head invokes typed producer, preregistration, evidence-contract,
analysis, and artifact checks through existing public owners, records absent generic result-packet
support as unavailable, replaces declarative statuses before --require-answerable, and rejects
incomplete artifact checksum declarations. Current proof is 55 focused tests, Ruff/format, docs
proof and evidence integrity, diff checks, and a diagnostic-only CLI smoke with no campaign.
This changes launch-gate and evidence-validity semantics, so Domain-Aware Approval and draft
disposition remain required. No merge-ready label, compute, campaign, evidence admission,
publication, or scientific result is authorized by this review.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
gate-verdict: accepted @ f06aa82
merge-ready: no
pr-metadata: reconciled @ 83b92cecda53d1cf036fd267ac77c888f1dca986114617519b6d75cda75d27e0
Exact-head implementation review accepted for the draft #7090 canonical answerability proof
composition. The refreshed head invokes typed producer, preregistration, evidence-contract,
analysis, and artifact checks through existing public owners, records absent generic result-packet
support as unavailable, replaces declarative statuses before --require-answerable, and rejects
incomplete artifact checksum declarations. Current proof is 55 focused tests, Ruff/format, docs
proof and evidence integrity, diff checks, and a diagnostic-only CLI smoke with no campaign.
This changes launch-gate and evidence-validity semantics, so Domain-Aware Approval and draft
disposition remain required. No merge-ready label, compute, campaign, evidence admission,
publication, or scientific result is authorized by this review.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
- Reviewed PR #7102 at authoritative remote head
f06aa823a12d5653427ad7001dfcd1a9583acbfeagainst currentorigin/main5c4f96468a2bacd3a04133df519f6d8756040073. - The refreshed tree includes the answerability proof-surface runner plus the fail-closed artifact-provenance fix: missing
answerability.artifacts.checksumsno longer raises a rawKeyError, and empty or malformed checksum declarations are rejected asinvalid_contract. - Focused answerability/manifest validation passed: 55 tests. Full
BASE_REF=origin/main scripts/dev/pr_ready_check.shpassed at this exact tree; changed-module coverage is 89.1%, reported as the repository's warning-only coverage-goal result. - Current exact-head hosted checks are terminal green: 29 passing checks and 2 intentional skips; older superseded reruns are not counted as current failures.
- The implementation is accepted for review, but
merge-ready: no: this remains a draft and domain-aware approval is still required for launch/evidence-admission behavior. - No benchmark, compute, scientific result, evidence admission, default change, or paper-facing claim was made.
gate-verdict: accepted @ f06aa823a12d5653427ad7001dfcd1a9583acbfe
merge-ready: no
|
Exact-head review for PR #7102.
Disposition remains non-merge-ready:
Friction follow-up #7112 tracks that the documented direct |
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Review gate
Accepted implementation proof for the exact current revision:
- Head:
e0bdc5e97f556af1dc6304068c6ab2541c176712 - Base:
e5acd8bff86a6c6266c6c8d4eb4f97564f9c80c7 - Focused answerability and manifest contract tests: 55 passed.
BASE_REF=origin/main scripts/dev/pr_ready_check.sh: passed; changed-module coverage 89.1% (warning-only), ratchets green.- Hosted checks for this head: 30 success, 2 skipped, no failures; the two cancelled contract runs were superseded by successful latest contract runs.
The implementation gate is accepted at this exact head. This remains merge-ready: no: the PR is draft and changes launch-gate/evidence-validity behavior, so explicit benchmark/domain approval is still required. No compute run, benchmark result, scientific interpretation, or merge authorization is inferred from this review.
## Summary Restore the executable mode for the repository's CI-status helper and extend its existing subprocess contract test to cover both direct and Python entrypoints. ## Linked Issues - Closes `#7112` ## Stack / Dependency - Base dependency: `origin/main` at `e5acd8bff86a6c6266c6c8d4eb4f97564f9c80c7` - Required prior PRs: none - Stack follow-up issues: none - Safe to review independently: yes - Review dependency reason, if any: this restores an existing script contract without changing CLI behavior ## What Changed - Restored `scripts/dev/check_pr_ci_status.py` from mode `100644` to executable mode `100755`. - Parameterized the existing help subprocess regression to prove direct shebang invocation and Python invocation both work with `PYTHONPATH` removed. - No script content or runtime behavior changed. ## Why It Matters - Added value: documented direct invocation no longer fails with `Permission denied` in fresh linked worktrees. - Expected impact: reliable exact-head CI inspection for maintainers and agent workflows. - Why this is worth merging now: the friction was observed during PR #7102 validation and affects every workflow that follows the documented direct entrypoint. ## Research Result Guidance - Target claim / hypothesis / blocker this should affect: NA - support/tooling friction only. - Comparator or baseline, if applicable: NA - no research or benchmark behavior changes. - Evidence tier: NA - Result classification: NA - Decision or stop rule, if applicable: NA - stop if either supported entrypoint fails its help contract. - Parent issue, claim map, registry, context note, or synthesis surface to update: #7112 only; no research claim surface is changed. - New research/benchmark/metric/paper-facing analysis tool, if any: NA - support helper; it only restores invocation of an existing CI-status utility. ## Domain-Aware Approval - Required for this PR: no - support helper with no evidence, benchmark, metric, or paper-facing claim. - Domains reviewed: NA - Status: not required - Approver/review source or waiver: not applicable - Validity checklist. Keep these machine-detected labels unchanged: - Target claim/hypothesis: NA - no research claim. - Comparator or split/evidence validity: NA - no comparison or evidence surface. - Fallback/degraded exclusions: NA - no benchmark execution path changed. - Claim boundary: support/tooling only; no CI success, merge-readiness, evidence admission, or benchmark validity is claimed by this change. - Implementation integrity vs experimental validity: implementation integrity is covered by focused tests; experimental validity is not in scope. ## Falsification / Non-Transfer Check - Did the mechanism activate? NA - support helper only. - Did the intervention change command source, selected command, trajectory, or route progress? NA. - Did the scenario actually contain the targeted failure mode? NA - no scenario executed. - Result route: NA. - Follow-up question or issue for weak, negative, or non-transfer results: NA. ## Next Empirical Action - Rerun needed: NA - no empirical action. - Extractor or analysis tool needed: NA. - Artifact missing or unavailable: NA. - Stop / revise / continue decision: continue normal CI-status workflow after merge. - Proposed child issue or existing follow-up: none. ## Validation / Proof - `scripts/dev/run_worktree_shared_venv.sh -- python -m pytest tests/dev/test_check_pr_ci_status.py -q` -> 59 passed. - `env -u PYTHONPATH scripts/dev/check_pr_ci_status.py --help` -> exit 0. - `env -u PYTHONPATH python scripts/dev/check_pr_ci_status.py --help` -> exit 0. - Ruff check and format check for the changed files -> passed. - `git diff --check` -> passed. - `PR_READY_MODE=interim BASE_REF=origin/main scripts/dev/pr_ready_check.sh` -> passed on exact head `0c4ac55818f1e5c922cfd4a52fe3e588c4f02348` against `e5acd8bff86a6c6266c6c8d4eb4f97564f9c80c7`, with core lane and ratchets green; no optional-extra lane was required. - The test asserts both entrypoints return help successfully and expose the expected CI-status options. - Benchmarks or smoke tests, if applicable: CLI help smoke only; no benchmark run. ## Risks / Rollout - Compatibility risks: none expected; the shebang and CLI implementation are unchanged. - Failure modes: an installation that strips executable mode would reintroduce the direct-path failure; the focused test makes that regression visible. - Rollback or fallback plan: retain the Python invocation while restoring the file mode in a follow-up if an environment-specific packaging constraint appears. ## Docs / Provenance - Updated docs: none; existing docs already describe the direct entrypoint. - Relevant design or provenance notes: issue #7112 records the observed permission failure. - Any assumptions that need to be preserved: the file retains its `#!/usr/bin/env python3` shebang and direct invocation is a supported repository path. ## Downstream Propagation - Parent issue updated (yes/no/NA): yes - exact-head handoff will be posted to #7112. - Claim map / benchmark report updated (yes/no/NA): NA - support helper only. - Leaderboard / artifact catalog updated (yes/no/NA): NA. - Registry or config index updated (yes/no/NA): NA. - Context index / memory note updated (yes/no/NA): NA. - Follow-up issue opened for deferred propagation (yes/no/NA): NA. - Not applicable because: this is a mode-bit and test-contract repair with no research artifact. ## Follow-Up Issues - Deferred work: none. - Issues opened for follow-up: none. ## Reviewer Notes - Anything a reviewer should verify closely: Git records the executable mode change and the test runs the real script path with `PYTHONPATH` removed. - Any known limitations: direct invocation still depends on the host having a usable Python 3 interpreter, as expected from the existing shebang. - Shared-helper migration: inapplicable; no helper behavior or call-site contract changed.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Review gate
Accepted implementation proof for the exact current-base revision:
- Head:
2297c14069fd50ebf5a453372f21db9e85952988 - Base:
65ebc834e68e5a19a5a8711c521913dbfa023262 - Focused answerability and manifest contract tests: 55 passed.
BASE_REF=origin/main scripts/dev/pr_ready_check.sh: passed with a clean stamp; changed-module coverage is 89.1% (warning-only), optional lane and all ratchets passed.- Hosted checks for this head: 32 total, 28 success, 2 skipped, 2 cancelled superseded runs, and no failures.
The implementation gate is accepted at this exact head. This remains merge-ready: no: the PR is draft and changes launch-gate/evidence-validity behavior, so explicit benchmark/domain approval is still required. No compute run, benchmark result, scientific interpretation, evidence admission, or merge authorization is inferred from this review.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head review for PR #7102.
- Reviewed head:
d6221267f7e2a000e82da114d3ffa7afd94288b7 - Base:
60124e812c693a8b14f0130ed7565f11588cc9ee - Authoritative linked worktree:
/home/luttkule/git/robot_sf_ll7.worktrees/pr-7102-review-20260815m - Focused answerability, proof-preflight, and manifest-runner suite: 52 passed.
- Final readiness: passed at the exact head with controlled four-worker core/optional lanes, ratchets, and clean-tree stamp
output/validation/pr_ready/review-pr-7102-current-20260815m.json. - Changed answerability-module coverage is 89.1% (49/55 executable lines), warning-only under the repository contract.
- CLI help, Ruff check/format,
git diff --check, docs/evidence integrity, docs-proof consistency, and evidence-registry ratchet: passed. - Hosted CI will be re-read after this review; no pending check is treated as proof.
- Body reconciliation digest:
357adbf16ac33e66cb8bc8b78e9581a7ffa1d9fbbcfba9b564c63ee41048e8a2.
The implementation is a fail-closed workflow/answerability contract. It composes canonical proof owners, records unavailable optional surfaces explicitly, rejects caveated success statuses, and keeps the diagnostic example non-answerable; it does not establish a planner, benchmark, scientific, paper, dissertation, or researcher-productivity result.
Domain-aware approval is explicitly required for campaign-admission proof behavior and remains pending. This review accepts implementation/provenance integrity only; #7090 remains open for maintainer/domain review and any further integration, and #7031/#7029 remain separate parent/packet surfaces.
No campaign, simulator episode, SLURM submission, external compute, evidence admission, publication, or issue closure was performed.
gate-verdict: accepted @ d622126
pr-metadata: reconciled @ 357adbf16ac33e66cb8bc8b78e9581a7ffa1d9fbbcfba9b564c63ee41048e8a2
merge-ready: no
|
Current-head audit for PR #7102 / issue #7090 (parent #7031):
Disposition: accepted implementation-integrity proof for canonical answerability admission and research-yield workflow composition. Domain-aware approval and any future campaign authorization remain external gates. This does not establish a planner, benchmark, scientific, paper, dissertation, or researcher-productivity claim, and no compute or evidence admission occurred.
|
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Terminal exact-head review: PR #7102 / issue #7090 / parent #7031
Reviewed the current implementation at exact head 0c5bf1c8e13c55f6076c6b023b1e937ed5031d71 against current origin/main=752e05f5574543bae4fbbde61be7ff79f501d9d9.
Implementation evidence:
- The linked worktree is clean and the diff remains limited to the nine intended answerability, manifest-runner, fixture, documentation, and test paths.
- Current focused answerability/preflight/manifest/contract validation passed
56tests. - The broader benchmark answerability/preregistration/yield filter passed
70tests on the refreshed tree. - The canonical example dry run produced four
diagnostic_onlyrows with required producer and analysis proofnot_runplus explicit unavailable optional proof surfaces. The same example with--require-answerablefailed closed with exit 2 andstate=diagnostic_only. - Ruff, formatting, docs/evidence integrity, proof/catalog consistency, evidence-registry ratchet
419/419, and diff checks passed. - Full
PR_READY_MODE=final PR_READY_ADVISORY=1 BASE_REF=origin/main scripts/dev/pr_ready_check.shpassed at2026-08-15T16:39:05.501333+00:00on this exact head and current base with a clean tree, core/optional lanes, and ratchets. Stamp:output/validation/pr_ready/review-pr-7102-current-20260815-v2.json. - Changed answerability-module coverage is
89.1%(49/55executable lines), warning-only under repository policy. - Hosted checks are terminal and healthy: 31 total, 29 successful, 2 intentional skips, 0 pending, 0 failed; 2 superseded historical runs are excluded by the queue classifier.
- Base freshness is fresh and the review-thread audit is complete with 0 unresolved threads.
- PR metadata is reconciled at
68dd2f70bf76d6364c3942aa30291187b6b90a5956f544ca84d278f0bf933735.
The implementation-integrity proof for canonical answerability admission and research-yield workflow composition is accepted for the recorded scope. Domain-Aware Approval remains pending for campaign-admission proof behavior, benchmark interpretation, evidence classification, experimental comparison, and paper-facing boundaries. The canonical example remains diagnostic-only; this review establishes no planner, benchmark, scientific, paper, dissertation, researcher-productivity, compute, or evidence-admission claim.
gate-verdict: accepted @ 0c5bf1c
pr-metadata: reconciled @ 68dd2f70bf76d6364c3942aa30291187b6b90a5956f544ca84d278f0bf933735
merge-ready: no
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
PR metadata/body reconciliation readback for #7102 / issue #7090.
- Final title:
feat(research): execute answerability proof surfaces. - Final exact head:
3ceec4f6c892410b0b0c25d26f794526e23c4d64. - Final base:
9b96aac9cf7a1a190662d6a499fcbbffbd0b8a77(origin/main). - REST body reconciliation succeeded and read back the requested title/body.
- Metadata digest:
3d86053b08bc42a09f27330fe17fa766f0cde90abd6994aaf8d783e66ebbf268.
pr-metadata: reconciled @ 3d86053b08bc42a09f27330fe17fa766f0cde90abd6994aaf8d783e66ebbf268
gate-verdict: accepted @ 3ceec4f6c892410b0b0c25d26f794526e23c4d64
merge-ready: no (fresh domain-aware approval and future campaign authorization remain external gates; no merge)
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head research-direction review for PR #7102 / issue #7090.
- Exact PR head reviewed:
3ceec4f6c892410b0b0c25d26f794526e23c4d64. - Exact base reviewed:
9b96aac9cf7a1a190662d6a499fcbbffbd0b8a77(origin/main). - The refreshed nine-file PR-owned diff composes the canonical research-answerability proof surfaces into the campaign-manifest path.
- Focused answerability/proof-preflight/manifest-runner suite: 56 passed.
- The default example run completed as
diagnostic_onlywith explicit unavailable optional proof surfaces;--require-answerablerejected it with underlying exit status 2. - CLI help, Ruff/format, diff checks, docs-proof consistency, docs/evidence integrity, and evidence-registry ratchet
419/419passed. - Final readiness passed at
2026-08-16T13:21:50.480925+00:00UTC on a clean tree; changed answerability coverage is 89.1%, warning-only. Stamp:output/validation/pr_ready/repair-pr-7102-live-refresh-20260816.json. - PR body metadata is reconciled:
pr-metadata: reconciled @ 3d86053b08bc42a09f27330fe17fa766f0cde90abd6994aaf8d783e66ebbf268.
gate-verdict: accepted @ 3ceec4f6c892410b0b0c25d26f794526e23c4d64
merge-ready: no (fresh domain-aware approval and future campaign authorization remain external gates; no merge)
This review establishes implementation and diagnostic answerability proof only. It does not authorize a campaign, compute, evidence admission, benchmark result, publication, or paper/dissertation claim.
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head research-direction review — PR #7102
- Reviewed head:
af658fb5ca1338dfd8e3eb9e5e791e847c2111d9. - Reviewed base:
462032df2abc3e086655935288c806b9df8bda2b(origin/main). - Rebased/merged cleanly onto current
main. The 9-path diff remains answerability proof-surface preflight composition. - Local validation: 52 focused tests passed. Ruff format and lint checks passed.
git diff --checkpassed. - Domain-aware approval: pending maintainer decision.
- Reconciled PR metadata:
pr-metadata: reconciled @ 76ff2a2bb60e78c6bf70a7452bcd1c26314786e1395d48bf0ddf98d4dcc6b3d5.
gate-verdict: accepted @ af658fb5ca1338dfd8e3eb9e5e791e847c2111d9
merge-ready: no (domain-aware approval pending)
Maintainer Decision RequiredWhat is complete:
What is missing for merge:
|
ll7
added
the
decision-required
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head review: not author-reserved
Reviewed head: af658fb5ccd87305628bb7925d9e523e5d321126; merge base 462032df2abc3e086655935288c806b9df8bda2b.
Why this is not an author-reserved hard stop. The diff is fail-closed validator and executor
tooling only. robot_sf/benchmark/research_answerability.py adds an optional proof_surfaces
section (_validate_proof_surfaces returns {} for legacy contracts, so pre-proof manifests are
unchanged) and one new strictly-narrowing state, blocked_missing_proof; it can only make a
contract less answerable, never more. scripts/validation/research_answerability_preflight.py
invokes existing public validators over argv lists with no shell and records unavailable surfaces
explicitly. Nothing in the diff writes a claim ledger, evidence-admission record, preregistration
authorization, catalog entry, release/tag, repository setting, or orchestrator-authority grant --
the only file writes in the changed set are tmp-path manifests inside tests. The canonical example
in configs/benchmarks/research_campaign_manifest.example.yaml stays diagnostic_only and is
rejected by --require-answerable with exit 2, and no workflow under .github/workflows/ invokes
the runner, so no benchmark row's evidence status changes as a result of merging this. The
"domain-aware approval" flag was a conservative self-classification by the implementation loop; the
uncertainty it named is reducible by reading the diff, and it resolves to "no author-reserved
surface touched".
Validation at this exact head.
- Hosted CI at
af658fb5ccd873: 31 contexts, all non-skipped checks green (fast-feedback1-4,
compat-matrix4x,determinism-gate,reproducibility-check,docs-evidence-integrity,
docs-link-integrity,wheel-smoke-install,examples-smoke,notebooks-smoke,ruff-security-advisory,
ty-advisory-ratchet, CodeQL); onlycoverage-gateandmerge-queue-gateskipped. scripts/dev/check_base_sensitive_gates.py --pr 7102 --json->gate_required: false
(nobase_sensitivefile intersection), so the ordinary current-main CAS path applies.- Unresolved review threads: 0.
- Diff verified against body claims: nine files, matching the declared inventory.
- Caveat: the body reconciliation performed as part of this review retriggered
pr-contract-check
andpr-body-contracts, which are queued behind ~100 other runs at the time of writing. They
validate PR metadata only; no code changed and the head SHA is unchanged. The guarded merger must
still confirm them green before merging.
Findings disposition (non-blocking).
- The previous body and gate-verdict trailers cited
af658fb5ca1338d..., which is not an object
in this repository. The trailer has been corrected to the real head. Tracking the general
defect separately. answerability_proofsupports acommandcheck kind that executes manifest-declared argv.
It is argv-only, shell-free, and opt-in behind--execute-validation/--require-answerable,
and it mirrors the pre-existingvalidation.commandsfield, so this is not a new trust boundary.
No campaign, compute, benchmark result, evidence admission, or paper-facing claim is produced by
this PR. Merge is not performed here.
gate-verdict: accepted @ af658fb5ccd87305628bb7925d9e523e5d321126
base-policy: ordinary-cas @ af658fb5ccd87305628bb7925d9e523e5d321126
pr-metadata: reconciled @ 5682d4a0f5c920a7f9f97f79279ffb048b58106d2699a6efacf85a3f408e0f44
compute: none
ll7
added
merge-ready
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head review (refreshed base): not author-reserved
Reviewed head: 40881aeed025e2441fa069bc2e6819563f606aa8; merge base a1892cf453973cd19e7bbba158a9f4132009bcee (current main).
This supersedes the earlier verdict at af658fb5ccd873, which is stale after the current-main refresh.
Why this is not an author-reserved hard stop. The diff is fail-closed validator and executor
tooling only. robot_sf/benchmark/research_answerability.py adds an optional proof_surfaces
section (_validate_proof_surfaces returns {} for legacy contracts, so pre-proof manifests are
unchanged) and one new strictly-narrowing state, blocked_missing_proof; it can only make a
contract less answerable, never more. scripts/validation/research_answerability_preflight.py
invokes existing public validators over argv lists with no shell and records unavailable surfaces
explicitly. Nothing in the diff writes a claim ledger, evidence-admission record, preregistration
authorization, catalog entry, release/tag, repository setting, or orchestrator-authority grant --
the only file writes in the changed set are tmp-path manifests inside tests. The canonical example
in configs/benchmarks/research_campaign_manifest.example.yaml stays diagnostic_only and is
rejected by --require-answerable with exit 2, and no workflow under .github/workflows/ invokes
the runner, so no benchmark row's evidence status changes as a result of merging this. The
"domain-aware approval" flag was a conservative self-classification by the implementation loop; the
uncertainty it named is reducible by reading the diff, and it resolves to "no author-reserved
surface touched".
Validation at this exact head.
- Refreshed onto current
mainby merge (no conflicts);git diff origin/main --statstill shows
exactly the nine intended files. - Focused suite rerun post-merge in a clean worktree:
52 passed
(tests/benchmark/test_research_answerability.py,
tests/validation/test_research_answerability_preflight.py,
tests/validation/test_run_research_campaign_manifest.py). ruff checkandruff format --check: clean.git diff --check origin/main: clean.scripts/dev/check_base_sensitive_gates.py --pr 7102 --json->gate_required: false
(nobase_sensitivefile intersection), so the ordinary current-main CAS path applies.- Unresolved review threads: 0.
- Hosted CI at the previous head
af658fb5ccd873was fully green across all 31 contexts. - Hosted CI at this head: 25 success, 1 skipped, 0 genuine failures, with
determinism-gate,
smoke-artifacts,reproducibility-check-reconciliation, and the PR-metadata contract checks
still queued behind a saturated runner pool (100+ queued repository runs). The twocancelled
entries are the first-pass PR-metadata checks superseded by the body reconciliation, not failures.
The guarded merger must confirm all required checks green at40881aeed02before merging;
scripts/dev/merge_queue_gate.pyfails closed onci_not_green, so this verdict cannot admit a
non-green head.
Findings disposition (non-blocking).
- The pre-review body and gate-verdict trailers cited
af658fb5ca1338d..., which is not an
object in this repository. Corrected here; the systemic defect is tracked in #7448. answerability_proofsupports acommandcheck kind that executes manifest-declared argv.
It is argv-only, shell-free, and opt-in behind--execute-validation/--require-answerable,
and it mirrors the pre-existingvalidation.commandsfield, so this is not a new trust boundary.
No campaign, compute, benchmark result, evidence admission, or paper-facing claim is produced by
this PR. Merge is not performed here.
gate-verdict: accepted @ 40881aeed025e2441fa069bc2e6819563f606aa8
base-policy: ordinary-cas @ 40881aeed025e2441fa069bc2e6819563f606aa8
pr-metadata: reconciled @ 89c0c8c0530795a78a9e442adcd25f58ea3d3e98abe73f34ac4c6a1fc4e6a308
compute: none
|
Superseded for delivery by PR #7469, which carries this proof-surface implementation together with the current answerability-to-camera-ready admission integration and the yield-dimension refresh. The older merge-ready label was removed to prevent duplicate or out-of-order admission tooling from merging; this PR remains open for traceability until #7469 completes its independent review and domain gate. |
PR reconciliation — superseded by #7469PR #7469 contains this PR's answerability proof collectors, manifest runner integration, fixtures, tests, and documentation, then adds the canonical camera-ready admission seam and bounded yield reporting. This branch remains useful as historical review evidence, but it is no longer an independent merge candidate. Close it to preserve one implementation owner and carry any unresolved proof-surface review findings to #7469. Canonical owner: #7469. |
) ## Summary Repair exact-head SHA provenance for PR bodies (#7448). PR bodies written by the autonomous loop routinely carried `gate-verdict: accepted @ <sha>` / `Exact head: <sha>` carriers naming SHAs that never existed in the repository. The merge gate already fails closed on such trailers; this change prevents the false evidence from being written or displayed in the first place. Adds one canonical parser for `gate-verdict`, `base-policy`, and `Exact head` SHA carriers in `scripts/dev/pr_loop_policy.py` (reusing the existing `GATE_VERDICT_RE` / `BASE_POLICY_RE`, no duplicate semantics). `scripts/dev/gh_pr_body_rest.py` now resolves the live PR `headRefOid` immediately before mutation and returns a structured fail-closed error before any PATCH when a carrier does not equal the live head; a best-effort `git cat-file -t` diagnostic classifies fabricated vs. stale-real-commit carriers, but equality to the live remote head is the admission rule. The same parser backs a new `analyze_sha_carriers` check in `scripts/dev/check_pr_followups.py`, so the `pr-body-contracts` workflow surfaces invalid carriers on already-written bodies (advisory, matching the workflow's existing mode). Scope is diagnostic-only tooling/provenance. `merge_queue_gate.py` gate semantics are unchanged (they already fail closed); merge authority (#7407/#7408) and metadata-conflict routing (#7405) are untouched. ## Validation - `scripts/dev/run_worktree_shared_venv.sh -- uv run pytest tests/dev/test_gh_pr_body_rest.py tests/dev/test_pr_loop_policy.py tests/dev/test_check_pr_followups.py -q` → 259 passed - `scripts/dev/run_worktree_shared_venv.sh -- uv run pytest tests/dev -k "pr_body and (gate_verdict or exact_head or sha)" -q` → 3 passed - `scripts/dev/run_worktree_shared_venv.sh -- uv run pytest tests/dev/test_merge_queue_gate.py tests/dev/test_snapshot_pr_queue.py -q` → 93 passed (consumers unchanged) - `scripts/dev/run_worktree_shared_venv.sh -- uv run pytest tests/dev -q` → 2032 passed, 3 skipped, 1 flaky unrelated slow-test failure (`test_marker_can_select_tests`, passes in isolation) - `uv run ruff check` + `uv run ruff format --check` on changed files → clean - `git diff --check` → clean - Parser smoke: fabricated SHA rejected, live head accepted, abbreviated rejected ## Audit of affected live PRs (report-only) | PR | body | ll7 comments | verdict | | --- | --- | --- | --- | | #7364 | 1 fabricated SHA (`74c9856d8ffc…`, missing locally) | — | false body claim remains | | #7336, #7307 | clean | — | clean | | #7260, #7236, #7152 | clean/valid | stale real-commit claims | stale comments | | #7102, #7104 | corrected (match live head) | stale real-commit claims | stale comments | No bodies edited, no comments posted, no labels changed. Only #7364 still carries a fabricated body claim; remediation (reconciliation + fresh exact-head review comment) is intentionally out of scope per the owner packet — the new guard prevents future fabrication. ## Evidence tier Tooling/provenance fix with regression fixtures for exact match, stale real commit, fabricated SHA, abbreviated SHA, quoted historical evidence, and concurrent head movement. No benchmark, planner, scientific, or paper-facing claim. Fixes #7448
## Summary Harden the `goal-pr-review` skill for the situation observed on 2026-08-18: many reviewer lanes plus the autonomous factory acting on the same PR set, a red shared `main`, `decision-required` labels that were mostly reducible, and PR bodies whose metadata was reconciled byte-wise but not truth-wise. Docs-only; no runtime behavior changes. ## Linked Issues - Relates to `#7448` (fabricated exact-head SHAs in gate-verdict trailers) - Relates to `#7491` (merge-ready + stale "not merge-ready" narrative reaching main) - Relates to `#7482` (shared-main namespace baseline that made unrelated PRs look red) ## Stack / Dependency - Base dependency: none - Required prior PRs: none - Stack follow-up issues: none - Safe to review independently: yes - Review dependency reason, if any: none ## What Changed - `.agents/skills/goal-pr-review/SKILL.md` (Codex mirror is a directory symlink; no separate copy): - **Concurrent Writers** section: read-live-before-mutate with an active-writer window; advisory `review-claim: <lane> @ <head> until <UTC>` marker; content-identical head moves (main refresh only) transfer findings but require re-publishing exact-head carriers after green CI; factory/owner label sweeps are authoritative ("one label away" reporting, no re-apply in-run); successor/superset detection across open PRs on the same issue. - **Shared-Main Baseline Before Per-PR Diagnosis**: run the base-sensitive marker suite on `origin/main` first; classify matching PR failures as `shared_main_blocked`; route one bounded repair instead of per-PR "fixes". - **Decision-Required Triage**: author-reserved taxonomy (claim/evidence admission, preregistration authorization, release/settings/secrets, authority *expansion* vs fail-closed narrowing) vs reducible; new `author_decision` parking state mapped to `AUTHOR_DECISION_REQUIRED`; ≤25-line decision-packet format; keep the branch mergeable while parked. - Step 4 body check: every 40-hex SHA in the body must resolve and equal live carriers (never prefix-complete); not-ready sentences must be re-narrated before `merge-ready`. - Step 9 + **Shared Resource Budget**: stop waiting under runner starvation and publish what exists; API-quota guard; no background pollers; worktrees on repo disk / free-space check; detached-HEAD inventory-test gotcha. - Output Requirements now include a terminal state, "one label away" detail, packet location, parked/racing writers. - `CHANGELOG.md`: Unreleased/Changed entry. ## Why It Matters - Added value: turns the ad-hoc rules ten parallel review agents had to be told out-of-band into the skill contract itself; each item is tied to an incident (evidence voided by mid-run rebases on 6 PRs; #7092/#7102 closed and a full `merge-ready` sweep while merges were queued; #7482 re-diagnosed per PR; 12 of 15 `decision-required` PRs proved reducible; #7374/#7435 bodies said "not merge-ready" while labeled/merged; API quota exhausted twice; `/dev/shm` full twice). - Expected impact: fewer racing writes, fewer wasted CI reruns, fewer author packets for reducible decisions, no unverifiable SHAs or stale narratives in squashed history. - Why this is worth merging now: the same review-drain pattern is running daily. ## Research Result Guidance - Target claim / hypothesis / blocker this should affect: NA — workflow docs only - Comparator or baseline, if applicable: NA - Evidence tier: docs-only - Result classification: NA - Decision or stop rule, if applicable: NA - Parent issue, claim map, registry, context note, or synthesis surface to update: none - New research/benchmark/metric/paper-facing analysis tool, if any: NA - support helper (skill contract text) ## Domain-Aware Approval - Required for this PR: no - docs-only skill contract; no evidence classification or claim surface changed - Domains reviewed: NA - Status: not required - Approver/review source or waiver: NA - Validity checklist. Keep these machine-detected labels unchanged: - Target claim/hypothesis: NA - Comparator or split/evidence validity: NA - Fallback/degraded exclusions: NA - Claim boundary: NA - Implementation integrity vs experimental validity: NA ## Falsification / Non-Transfer Check NA — no empirical claim. ## Next Empirical Action NA. ## Validation / Proof - `uv run python scripts/dev/check_skills.py` → Validated 55 skills, typed registry, generated README, and routing tests. - `uv run pytest tests/dev/test_check_skills.py tests/dev/test_factory_v2_skill_contract.py tests/dev/test_token_efficient_thread_profile_snapshot_command.py -q` → 64 passed, 1 skipped. - `uv run python scripts/tools/sync_ai_config.py --check` → 7 symlinks validated. - `uv run python scripts/dev/check_docs_evidence_integrity.py --files …` → 2 changed files passed. - `uv run pre-commit run --files .agents/skills/goal-pr-review/SKILL.md CHANGELOG.md` → passed. ## Performance Evidence NA — docs only. ## Risks / Rollout - The `review-claim` marker is advisory until tooling (factory / `pr_loop_policy.py`) recognizes it; the text says so. - `author_decision` is a documented parking state, not a new `pr_loop_policy.py` classification; a follow-up could add machine detection of `decision-required` + packet presence. ## Docs / Provenance - Skill contract updated in place; CHANGELOG entry added. ## Downstream Propagation NA — not an evidence-producing PR. ## Follow-Up Issues - Optional: teach `pr_loop_policy.py` / merge gate to recognize the `review-claim` marker and the not-ready-sentence check (#7491 already tracks the gate side). ## Reviewer Notes Diff is additive apart from one table row (`failed_ci` mapping) and the Output Requirements list. Long lines are inside Markdown table rows, matching the existing file style. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Refs #7090, #7031.
Connect the existing
research_answerability.v1contract to the canonical campaign-manifestvalidation path. This preflight executes or explicitly records unavailable producer,
preregistration, evidence-contract, analysis, artifact, and result-packet proof surfaces, and
--require-answerablefails closed unless the evaluated contract is decision-capable.The change reuses existing answerability, preregistration, evidence-contract, artifact-catalog,
and result-packet owners. It does not add a second artifact registry, generic PR-ready gate,
campaign router, or launch authorization path.
Research and Evidence Boundary
answerableunless its producer-to-estimand-to-analysis-to-artifact chain is executable or an explicit required proof is unavailable.diagnostic_only; it is not a benchmark result.--require-answerable; no campaign or evidence admission is authorized here.Changes
Domain-Aware Approval
answerable.diagnostic_only;--require-answerablerejects it with exit 2, and no benchmark or paper-facing claim is established.Validation / proof
40881aeed025e2441fa069bc2e6819563f606aa8.a1892cf453973cd19e7bbba158a9f4132009bcee(origin/main).52 passed.--require-answerable; no packet or campaign result is admitted.git diff --check: passed.mainvia merge commit (no conflicts); focused suite rerun after the merge.Downstream propagation
gate-verdict: accepted @ 40881aeed025e2441fa069bc2e6819563f606aa8merge-ready: yes