Fix screening decision order, narrow manifest re-binding, unblock nested Codex critic - #3
Open
aarontaycheehsien wants to merge 4 commits into
Open
aarontaycheehsien wants to merge 4 commits into
aarontaycheehsien wants to merge 4 commits into
Conversation
…ition implied_decision walked required criteria in order and returned "uncertain" at the first unsettled one, so an evidenced "no" on a later criterion was never reached. The rubric (references/candidate-screening.md) makes exclude "at least one decisive failure" and uncertain "everything else": a required "no" excludes regardless of order. Found in the CD010657 pilot, where the validator rejected a correct exclude as contradicting its verdicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… path Screening worksheets and provenance-blinded pilot rounds are written blank and completed in place, so the gate flagged their earlier entries' hashes (seen in the CD010657 pilot). A later `add --output P --supersedes P` now re-binds P, but only when P is one of those artifacts (operation screening-worksheet or orthogonal-pilot-screening). Honouring it for any path let an edited final QA, search, validation, critic, or audit output pass the hash check: a failing QA edited to "ok": true and re-bound to itself was no longer reported. The CD010657 pilot manifest validates with no hash findings under the narrowed rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In the Phase 3 build codex exec exited 1 with an empty final message and empty stderr; the reason (a ChatGPT usage limit) was only in the event stream as a turn.failed event, so generate.py reported just "non-zero exit code". It now prints Codex's last reported error and writes failure.json (stage, returncode, timed_out, codex_error, elapsed_seconds) to the run dir. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…agent env
Found in the Phase 3 acceptance build:
- A Codex child nested in a Codex sandbox has most shell commands refused
("blocked by policy"; unelevated fails CreateRestrictedToken), and shell
is its only way to read files. The critic returned EVIDENCE-ACCESS and a
re-screen answered only the records it happened to see. The codex-cli
runner now inlines every staged file verbatim in the prompt (refusing,
not truncating, over 2 MB). Verified nested: the child returns a staged
file's exact contents.
- CLAUDECODE from the launching Claude Code session reached the Codex build,
so the skill's auto runner picked Claude Code. The eval driver now starts
Codex without CLAUDECODE / CLAUDE_CODE_* variables.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eff52f1ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+462
to
+464
| if artifact_key(out_value) == artifact_key(sup_value) and is_fill_in_place_artifact(base_path, out_value): | ||
| key = artifact_key(out_value) | ||
| rebound_at[key] = max(rebound_at.get(key, 0), seq_value) |
There was a problem hiding this comment.
The exemption trusts the artifact's current, already-edited operation value, so the laundering path remains open: an edited QA, critic, or other JSON artifact can add "operation": "screening-worksheet", then be self-superseded, causing every earlier hash mismatch for that path to be skipped while the new entry matches the edited file. Re-binding eligibility must be established from immutable metadata captured when the original artifact was recorded, rather than from the current file whose integrity is being checked.
Useful? React with 👍 / 👎.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2 (merged). Four commits made after that merge: a screening-decision fix found during a CD010657 pilot review, a narrower version of the manifest re-binding fix (the version in #2 could be laundered), and two fixes found while running the Phase 3 acceptance build.
Type of Change
Related Issue
Follow-up to #2.
Changes Made
scripts/screening_tool.py):implied_decisionwalked required criteria in order and returneduncertainat the first unsettled one, so an evidencednoon a later criterion was never reached.references/candidate-screening.mddefines exclude as "at least one decisive failure" and uncertain as "everything else" — a requirednoshould exclude regardless of order. Found reviewing the CD010657 pilot, where a correct exclude was rejected as contradicting its verdicts.scripts/manifest_tool.py): Audit and harden the eval harness; unblock the sandboxed independent critic #2 letadd --output P --supersedes Pre-bind any path filled in place (needed for screening worksheets, which are written blank and completed later). That was too broad: an edited final QA, search, validation, critic, or audit output could be re-bound to itself and its hash-mismatch finding would disappear. Re-binding is now allowed only for artifacts whoseoperationisscreening-worksheetororthogonal-pilot-screening. A regression test re-creates the laundering (a failing QA edited to pass, then re-bound) and confirms it's still caught; the CD010657 pilot manifest still validates cleanly.evals/generate.py): a failedcodex execcan exit 1 with an empty final message and empty stderr — the reason (seen live: a ChatGPT usage-limit message) is only in the JSONL event stream.generate.pynow reads the lasterror/turn.failedevent and prints it, and writes afailure.json(stage, returncode, timed_out, codex_error, elapsed_seconds) into the run directory.scripts/isolated_runner.py,evals/drivers/codex.py): found live during the Phase 3 acceptance run.EVIDENCE-ACCESSfinding instead of reviewing, and an independent re-screen answered only the records it happened to see. Thecodex-clirunner now inlines every staged file into the child's prompt verbatim, so the child needs to run no commands. Evidence larger than 2 MB is refused, never silently truncated. Verified live, nested in the sandbox: the child returned a staged file's exact contents.CLAUDECODEfrom the launching Claude Code session, so the skill's own "auto" isolated-runner selection picked Claude Code instead of Codex inside the build under test. The driver now starts the agent under test withoutCLAUDECODE/CLAUDE_CODE_*variables.Testing
python -m unittest discover -s tests(how CI runs it): 977 passed.python scripts/pubmed_tool.py doctorreportsok: true.Checklist
pubmed_tool.py doctorReviewer notes
protocol-first-empirical-search-builder, matching Audit and harden the eval harness; unblock the sandboxed independent critic #2 and the branch-divergence note (mainhas noscreening_tool.py/isolated_runner.py/critic_tool.pyfor these fixes to apply to).generate.pyrun with a live critic before/after ablation has not been completed yet.🤖 Generated with Claude Code