feat(llm): preflight model pilot candidates - #2769
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pilot enriches verifier inputs with linked issue context, caches case fetches across candidates, runs candidate preflight validation, aggregates category-level results, and publishes full-run and preflight reports. ChangesModel evaluation pilot calibration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant run_preflight
participant run_pilot
participant GitHub
participant evaluator
CLI->>run_preflight: execute representative case
run_preflight->>run_pilot: run reduced corpus
run_pilot->>GitHub: fetch and cache PR context
run_pilot->>evaluator: evaluate candidate
evaluator-->>run_pilot: return result row
run_pilot-->>run_preflight: return preflight report
CLI->>run_pilot: execute full corpus after validation
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🤖 Keepalive Loop StatusPR #2769 | Agent: Codex | Iteration 0/12 Current State
Last Codex Run
To retry immediately:
Or wait for the next successful Gate run to automatically retry. 🔍 Failure Classification| Error type | infrastructure |
|
Keepalive Work Log (click to expand)
|
|
Runner dispatch state for codex on PR #2769. Do not edit. |
There was a problem hiding this comment.
Pull request overview
This PR improves the LLM verifier model-evaluation pilot by enriching each case’s prompt with linked source-issue context, adding a one-case-per-candidate preflight stage, and emitting an aggregated category-level summary alongside the raw per-row results.
Changes:
- Extend
fetch_pr()to include linked source issue bodies plus recent verifier/disposition comments in the evaluation context. - Add a candidate preflight run and cache per-case GitHub fetches so each case is fetched once across all candidates.
- Add a
summarysection to the pilot results and surface it in the workflow step summary; upload the preflight artifact.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/run_model_eval_pilot.py | Adds linked-issue context enrichment, preflight stage, per-case fetch caching, and a computed summary in the pilot output. |
| tests/tools/test_run_model_eval_pilot.py | Adds coverage for linked-issue context enrichment, preflight behavior, fetch caching, and summary aggregation. |
| .github/workflows/maint-78-model-evaluation-pilot.yml | Updates the run summary to include the new summary payload and uploads the preflight artifact. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91912fa77c
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/maint-78-model-evaluation-pilot.yml (1)
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreflight failure diagnostics aren't surfaced when the full pilot never runs.
When preflight fails (
main()returns 1 beforepilot-results.jsonexists), the summary falls back to a generic message even thoughpilot-preflight.json(with the specific unusable candidates) is already on disk at that point. Surfacing it here would speed up triage.💡 Fall back to the preflight report when the full-run artifact is missing
if [ ! -f pilot-results.json ]; then echo '## Verifier model pilot' >> "$GITHUB_STEP_SUMMARY" - echo 'The pilot failed before producing a results artifact; inspect the run step.' \ - >> "$GITHUB_STEP_SUMMARY" + if [ -f pilot-preflight.json ]; then + echo 'Preflight failed before the full pilot ran:' >> "$GITHUB_STEP_SUMMARY" + echo '```json' >> "$GITHUB_STEP_SUMMARY" + jq '{schema, candidates: .summary.candidates}' pilot-preflight.json \ + >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + else + echo 'The pilot failed before producing a results artifact; inspect the run step.' \ + >> "$GITHUB_STEP_SUMMARY" + fi exit 0 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/maint-78-model-evaluation-pilot.yml around lines 33 - 38, Update the pilot-results.json fallback block in the workflow to check for pilot-preflight.json when the full-run artifact is missing. If present, append a fenced JSON summary containing its schema and summary.candidates to GITHUB_STEP_SUMMARY; otherwise retain the existing generic failure message, then exit successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/tools/test_run_model_eval_pilot.py`:
- Around line 101-124: Add a negative-path test alongside
test_fetch_pr_includes_linked_source_issue_context that makes fetch_issue or
fetch_issue_comments raise, then assert fetch_pr still returns the PR context
and diff without propagating the enrichment failure. Use the existing
pilot.api_client mocks and verify the linked-issue enrichment is omitted or
safely degraded while the core PR result remains available.
In `@tools/run_model_eval_pilot.py`:
- Around line 225-239: Wrap the run_preflight call in main() with a ValueError
handler so empty or malformed corpus validation failures are reported cleanly to
stderr and main() returns 1, matching the existing token and preflight failure
behavior. Keep successful preflight processing and unusable-candidate handling
unchanged.
- Around line 21-29: Update _linked_issue_numbers to recognize relationship
phrases such as “closes,” “fixes,” “resolves,” and “related to” when they appear
inline, while preserving case-insensitive matching, deduplication order, and
exclusion of pr_number. Add focused tests covering duplicate references and
self-references, including inline occurrences, rather than relying only on
test_fetch_pr_includes_linked_source_issue_context.
- Around line 37-46: Update the linked-issue loop around _linked_issue_numbers
and api_client.fetch_issue so each issue fetch and its related comment retrieval
are wrapped in a narrow try/except. On fetch, API-shape, or comment errors, skip
that issue and continue processing the remaining linked issues, while preserving
disposition_comments behavior for successful issues.
- Around line 39-72: Limit and sanitize linked source-issue bodies and
disposition comments before appending them to source_issues and context in the
model-evaluation flow. Apply a bounded length to each issue body/comment and
enforce an overall linked-issue context budget, preserving the existing issue
metadata while preventing attacker-controlled threads from dominating the
verifier prompt.
---
Outside diff comments:
In @.github/workflows/maint-78-model-evaluation-pilot.yml:
- Around line 33-38: Update the pilot-results.json fallback block in the
workflow to check for pilot-preflight.json when the full-run artifact is
missing. If present, append a fenced JSON summary containing its schema and
summary.candidates to GITHUB_STEP_SUMMARY; otherwise retain the existing generic
failure message, then exit successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b6005ef-b684-4293-82f0-f317cecdd6bb
📒 Files selected for processing (3)
.github/workflows/maint-78-model-evaluation-pilot.ymltests/tools/test_run_model_eval_pilot.pytools/run_model_eval_pilot.py
🤖 Bot Comment Handler
The agent has been assigned to this PR to address the bot review comments. Instructions for agent
The bot comment handler workflow has prepared context in the artifacts. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tools/run_model_eval_pilot.py (2)
282-296: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message text no longer matches the failure condition it reports.
unusable_candidatesnow flags candidates missing schema-valid rows for some expected cases, not necessarily "no schema-valid rows" (a candidate could be 29/30 valid and still trip this). The stderr message on both failure paths still says "produced no schema-valid rows," which will mislead whoever triages a CI failure.✏️ Align wording with actual semantics
- "pilot preflight error: candidates produced no schema-valid rows: " + "pilot preflight error: candidates missing schema-valid rows for one or more corpus cases: " + ", ".join(unusable), file=sys.stderr, ) return 1 payload = run_pilot(corpus, candidates, token=token) args.output.write_text(json.dumps(payload, indent=2) + "\n") unusable = unusable_candidates(payload) if unusable: print( - "pilot error: candidates produced no schema-valid rows: " + ", ".join(unusable), + "pilot error: candidates missing schema-valid rows for one or more corpus cases: " + + ", ".join(unusable), file=sys.stderr, ) return 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/run_model_eval_pilot.py` around lines 282 - 296, Update both stderr messages in the preflight and post-run branches of the main evaluation flow to describe candidates missing schema-valid rows for one or more expected cases, rather than claiming they produced none. Keep the existing unusable candidate list and failure handling unchanged.
54-88: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBudget check runs after the network calls it's meant to prevent.
remaining_context <= 0is only checked afterfetch_issue/fetch_issue_commentsalready ran for that issue. Once the budget is exhausted, every subsequent linked issue still pays for 2 GitHub API calls before being discarded onbreak.♻️ Check the budget before fetching
for issue_number in _linked_issue_numbers(pr_body, pr_number=number): + if remaining_context <= 0: + break try: issue = api_client.fetch_issue(repo, issue_number, token) ... except Exception as exc: print(f"pilot: skipping linked issue #{issue_number}: {exc}", file=sys.stderr) continue - if remaining_context <= 0: - break source_issues.append(source_issue[:remaining_context]) remaining_context -= len(source_issues[-1])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/run_model_eval_pilot.py` around lines 54 - 88, Move the remaining_context <= 0 guard in the linked-issue loop so it executes before fetch_issue and fetch_issue_comments are called. Keep the existing source_issues append and budget decrement behavior unchanged for issues fetched while budget remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/run_model_eval_pilot.py`:
- Around line 83-84: Update the exception handler in the per-issue processing
flow to emit a diagnostic log containing the failed issue context and exception
details before continuing. Preserve the existing continue behavior so one issue
failure does not stop processing the remaining issues.
---
Outside diff comments:
In `@tools/run_model_eval_pilot.py`:
- Around line 282-296: Update both stderr messages in the preflight and post-run
branches of the main evaluation flow to describe candidates missing schema-valid
rows for one or more expected cases, rather than claiming they produced none.
Keep the existing unusable candidate list and failure handling unchanged.
- Around line 54-88: Move the remaining_context <= 0 guard in the linked-issue
loop so it executes before fetch_issue and fetch_issue_comments are called. Keep
the existing source_issues append and budget decrement behavior unchanged for
issues fetched while budget remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99ec09b1-5abc-463f-b8ea-8da34a7d5140
📒 Files selected for processing (3)
langsmith-fleet-worker-attempt.jsontests/tools/test_run_model_eval_pilot.pytools/run_model_eval_pilot.py
|
Addressed the remaining CodeRabbit thread in 1c55b31: linked-source failures now emit a bounded diagnostic and the context budget guard runs before additional API calls. Focused pilot tests (13), Ruff, Black check, and diff check pass. |
Closes #2768
Automated Status Summary
Scope
Scope section missing from source issue.
Context for Agent
Related Issues/PRs
Tasks
Acceptance criteria
Summary by CodeRabbit