Repository navigation
Make nonpaid SkillEval plans actionable for PR authors - #292
Conversation
Signed-off-by: Codex <codex@openai.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Skill evaluationAutomatic execution / cumulative PR budget: Advisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
PLANNED: execution is not authorized by knowing this hash. Selected comparisons: 0 · Estimated task calls: 0. Applicable only while these revisions and inputs match the current PR. Plan bindings, budget, frozen exam and advisory detailsUnchanged-correct is valid neutral evidence, not incremental benefit. Per-case outcomes (discovery excluded; regressions and gaps first): []Showing at most 20 of 0 pairs; the evidence link contains all outcomes, including omitted regressions/gaps. Plan SHA-256: Model/settings:
Provider/settings: Uncovered / unsupported paths (6)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52971cc818
ℹ️ 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".
| if len(pins) != 1 or git(repo, "cat-file", "-t", pins[0]).strip() != b"commit": | ||
| return unavailable |
There was a problem hiding this comment.
Make projection-check metadata independent of checkout depth
When the declared source pin predates the base checkout, this lookup produces different plans in the existing workflow's jobs: .github/workflows/skill-eval-pr.yml uses a default shallow checkout for plan, but fetch-depth: 0 for publish. The fetch in main() does not remove the shallow boundary, so the planner records projection_check.status = unavailable while the publisher rebuilds it as ready for identical PR revisions. I reproduced the missing ancestor with the workflow-equivalent fetch. Because this metadata enters the plan fingerprint, source_plan_expiry() rejects the original execution plan and retained results become invalid rather than publishable. Ensure both jobs have the same source objects before fingerprinting, or keep availability-dependent display metadata outside the execution binding; git fetch -h documents --unshallow as “convert to a complete repository.”
Useful? React with 👍 / 👎.
Align plan and publish history and restrict automatic source-pin selection to the frozen head ancestor closure. Preserve evidence and execution gates. Signed-off-by: Codex <codex@openai.com>
Dong1017
left a comment
There was a problem hiding this comment.
Scoped software LGTM for 297835b34d22add47a1a458cdb883f0d67f2c939 against 33de003e; no remaining substantive finding in the six-file change.
The guidance now gives a pinned, read-only index-check command where justified, identifies mapped task purpose/trigger/required outputs, and asks for explicit evidence and unverified-claim dispositions. Generated path counts are not presented as independent claims; unsupported snapshots and absent metadata remain explicit.
Prior P2 disposition: 4432063862 is addressed by matching full-history plan/publish checkouts and restricting automatic source-pin suggestions to the frozen head’s ancestor closure. I independently reproduced both the shallow/full mismatch and an unrelated-ref availability variant; the new regressions establish equal plans/fingerprints after correction. Missing/unrelated pins remain unavailable—not silently ready. The historical thread is outdated but still unresolved in the UI; this is a substantive disposition, not a claim that its UI state changed.
Evidence and limits
- Published tree
12daa787and all six blobs match the independently reviewed local source. Final independent focused checks: 38 passed; author’s affected suite: 249 passed. - Ready 41211932585: all five jobs succeeded. Latest declarations passed; current-head native review reports no findings. Corrected body/head declaration was read back.
- Reviewed bounded Git-data reads, command construction, escaped metadata and fixed public error categories; raw historical plans remain unchanged. No mapping, grader, paid gate or snapshot-safety relaxation.
- The #90/#263 examples are data-only previews, not executed checks or rebinding of historical results. The current bot comment still uses trusted base
33de003e, not this undeployed renderer. This clears software guidance—not an actual author/agent loop, task applicability for every claim, or efficacy; #253’s remaining evidence/disposition acceptance stays open.
COMMENT review only; no formal approval or experiment authorization.
Scope
Refs #253 and the accepted #215 planning/advisory boundary. Make nonpaid PLANNED feedback useful to PR authors without changing task selection or execution policy.
Verification and limits
Final local verification: 249 affected tests passed; whole-tree Ruff and diff checks passed. A prior fixed snapshot before the final safe-reason display projection passed 3,257 tests, 12 skipped, 284 subtests; that aggregate is not claimed for the final head. Independent review covered the display boundary and the final history-stability correction, including 38 focused tests. Normal current-head repository CI remains required.
Data-only previews reuse the exact retained #90/#263 plans plus independently fetched public source metadata: both gain an actual pinned index-check command; #263 also names its snapshot rejection and the diagnostic task's purpose/required outputs. The compact front screen is about 1.1k characters, with the full audit retained. These previews are not executed index checks, a refresh of historical bot comments, re-bound trial evidence, a real author-agent loop or measured skill benefit. No model/provider/hardware execution; no mapping, grader, budget, credential or acceptance-gate changes. The only workflow change aligns plan checkout to the existing publisher with
fetch-depth: 0; execute, permissions and credentials are unchanged. #253's remaining actual author evidence/disposition requirements stay open.Native-review correction
The native P2 identified that shallow planning and full-history publication could derive different source-pin metadata/fingerprints. The correction aligns both history depths and restricts automatic command suggestions to the frozen head’s ancestor closure; unrelated cached/missing objects now give the same explicit unavailable result. This is display eligibility, not a failed source check or a new acceptance gate. Real local file-transport regressions cover both shallow/full parity and deletion of an unrelated source branch. Original evidence remains unchanged; current-head CI is still required.
Authorship
Actual implementation and independent review: Codex / gpt-6-astra / xhigh.