feat(tier3): rebuild ATIF from OpenCode and Codex agent logs - #119
mimran-khan wants to merge 21 commits into
Conversation
Parse OpenCode JSON stream events into synthetic trajectories when trajectory.json is empty, and reuse the parser for structured Codex tee logs. Fail closed on non-dict tool inputs. Fixes NVIDIA#118 Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Codex tee logs use type=item / agent_message events, not OpenCode tool_use. Add a dedicated parser with stderr-tolerant JSONL scanning, keep OpenCode-shaped JSONL as a fallback, and normalize shell calls to bash for skill checks. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
|
Codex parser fixes are pushed from the earlier round. Ready for re-review when you have time. |
rng1995
left a comment
There was a problem hiding this comment.
The current head still does not reconstruct real Codex exec events, and the new OpenCode fallback is not connected to the collected-results discovery path. Focused converter and refinement tests passed (95 tests), with Ruff and diff checks clean. Requesting changes for the two reproducible integration gaps noted in the review threads.
…ries Merge main and handle Codex exec JSON item.completed events with agent_message text and command_execution items. Include opencode in trajectory discovery so refinement can rebuild ATIF from opencode.txt when trajectory.json is missing.
…an-khan/SkillEvaluator into feat/opencode-atif-fallback
Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Resolve trial case ids from reward/result metadata before refine lookup. Parse Codex file_change and mcp_tool_call events, and keep OpenCode tool failures in observations via state.error. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
rng1995
left a comment
There was a problem hiding this comment.
Three earlier integration issues are fixed and their threads are resolved. The focused converter/refinement suite passes (36 tests), with Ruff and diff checks clean. Two correctness gaps remain: Codex terminal status, exit code, and some error forms are discarded, and OpenCode failures that include partial output lose both failed status and error details. These make failed actions indistinguishable from successful empty or partial-output actions in downstream evidence, so I am requesting changes on the two remaining threads.
Preserve exit code, status, and error evidence for failed command runs, file changes, and MCP calls, and keep OpenCode error text alongside partial output for downstream grading. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Keep Harbor trial discovery tests from main and restore OpenCode trajectory fixtures from this branch. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
|
Pushed b4644da with main merged in. Codex command_execution now keeps status, exit_code, and errors even when command is false or output is empty; file_change and MCP calls get the same terminal evidence, and OpenCode keeps status plus error text alongside partial output. Harbor trial discovery tests from main are in, plus the OpenCode trajectory fixtures on this branch. Happy to re-test anything specific. |
|
@mimran-khan, two code blockers remain at
Please address both items and ping me when the PR is ready for re-review. |
Attach file-change status to every write call, and trim long tool output without dropping status or error text. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
|
@rng1995 when you have a moment, could you re-review from the current head? I do not have permission to re-request reviewers on the upstream repo from my account. |
rng1995
left a comment
There was a problem hiding this comment.
Re-reviewed af17bad. Both outstanding terminal-evidence issues are fixed with focused regressions: multi-file changes retain failure evidence for every emitted write, and bounded long-output observations retain terminal status and error details. The focused and broader Tier 3 suites pass, Ruff and diff checks are clean, the PR is conflict-free, and all 17 GitHub checks, including DCO, are green. Approved.
|
@chrisknvidia - Please review and approve the PR. |
| steps.append(step) | ||
| continue | ||
|
|
||
| if item_type == "mcp_tool_call": |
There was a problem hiding this comment.
[P2] Preserve completed Codex web_search items. This dispatch ends after mcp_tool_call, so current web_search ThreadItems are silently skipped. In a retained 20-log replay, all 46 completed searches produced zero synthetic tool calls, while paired native trajectories retained exactly 46 web_search_call entries. That removes source-retrieval behavior whenever the missing/empty-trajectory fallback is used. Please map the query/action payload and cover it with a captured Codex event fixture.
There was a problem hiding this comment.
The branch is present now, but the captured-schema issue remains. Current codex exec --json WebSearchItem carries an action object (search, open_page, or find_in_page); it does not carry the synthetic status/output fields used by the new test. An open_page event therefore loses its URL/action and emits only the query. Please map item.action into the ATIF arguments and replace the fixture with a schema-valid captured event.
There was a problem hiding this comment.
@rng1995 addressed on latest push: WebSearchItem now maps item.action (search/open_page/find_in_page) into ATIF arguments with captured-schema fixtures, and collab_tool_call items (spawn_agent, etc.) synthesize into tool steps. Merged main again. Ready for re-review.
There was a problem hiding this comment.
@mimran-khan — Re-tested the current 8063d5b head against the cited Codex schema; this remains open. WebSearchAction::Search supports both optional query and queries, while the serialized WebSearchItem carries a required top-level query and no status. The new tests omit that top-level query, inject status, and cover only singular action.query, so they are not captured-schema fixtures. A schema-shaped event with action={"type":"search","queries":["alpha","beta"]} currently yields action_input={"query":"parallel query batch"}: the action type and both actual queries are dropped from the ATIF arguments (they survive only as observation text). Please preserve the complete action payload in action_input and cover schema-valid search, open-page, and find-in-page events.
Merge main, prefer task_id.path for Harbor entry ids, recognize Codex terminal status in error recovery, and synthesize web_search thread items. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
|
@chrisknvidia follow-up on todays three threads: |
| steps.append(step) | ||
| continue | ||
|
|
||
| if item_type in {"web_search", "web_search_call"}: |
There was a problem hiding this comment.
[P2] Preserve completed Codex collaboration calls. The current codex exec --json ThreadItem schema also emits collab_tool_call items (spawn_agent, send_input, wait, and close_agent), but this dispatch skips them. A log containing only a completed spawn-agent item returns None, and mixed logs silently omit the call. Please map its tool/prompt/receiver/status payload into ATIF and add a schema-valid captured fixture.
There was a problem hiding this comment.
Same push adds collab_tool_call handling with a spawn_agent fixture test. Thanks.
There was a problem hiding this comment.
@mimran-khan — Re-tested the current 8063d5b head against the cited CollabToolCallItem schema; this remains open. The wire fields are tool, sender_thread_id, receiver_thread_ids, prompt, agents_states, and status. _codex_collab_tool_arguments copies prompt but looks for unrelated singular keys such as receiver and agent_id, so a schema-shaped spawn event is reduced to action_input={"prompt":"review logs"} and loses sender, all receivers, and agent states. The added fixture omits those required wire fields, so it does not exercise the requested receiver/status payload. Please map the exact schema fields and add a schema-valid captured fixture.
|
@mimran-khan Let us know when you have addressed the final round of review comments present in PR. Post that, I can help merge the changes. Thanks for your contributions! |
Missed the last update for some reason, let me take a look. @rng1995 : thanks for the nudge |
Merge main and synthesize WebSearchItem action payloads plus collab_tool_call thread items into ATIF with schema-shaped fixtures. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
rng1995
left a comment
There was a problem hiding this comment.
@mimran-khan — Re-reviewed current head 8063d5b. The earlier Codex/OpenCode integration, Harbor case-id, terminal-evidence, bounded-output, and downstream error-recovery findings are fixed; their verified threads are resolved. Local verification is otherwise clean: changed-file Ruff passes, focused converter/refinement tests are 53/53, the broader Tier 3 suite is 1,235 passed, and git diff --check is clean. Two schema correctness blockers remain in the existing unresolved threads: (1) schema-valid multi-query web_search events lose the action type and queries from ATIF arguments, while the new fixtures omit required wire fields and add non-wire status; (2) schema-valid collab_tool_call events lose sender_thread_id, receiver_thread_ids, and agents_states, and the new fixture does not exercise them. These can silently remove source-retrieval and delegated-agent evidence, so I am requesting changes. The newest fork workflows also remain at the separate action_required authorization gate.
Fixes #118
When Harbor leaves
trajectory.jsonempty, Tier 3 scoring used to fail with "No trajectory or reconstructible agent log" even though OpenCode and Codex tee structured JSON toopencode.txtandcodex.txt.This adds parsers for OpenCode
run --format=jsonevents (text + completedtool_userecords) and wires them intoload_trajectory_with_fallbackafter the existing claude/cursor/cline fallbacks. Codex reuses the same parser when every tee line is JSON. Non-dict tool inputs are ignored so raw strings are not treated as shell commands.Harbor task bundles already copy
log_converters.pythroughadapter.py, so generated verifiers pick this up automatically.Distinct from #110, which is about exec steps inside an existing trajectory.
Test plan
pytest tests/tier3/test_log_converters.pypytest tests/test_harbor_collector_runtime_failures.py