Repository navigation
fix(opencode): include cached input tokens - #4106
Merged
Merged
Conversation
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
giuliolovisotto
added
bug
yaoyu-33
added
the
complexity:low
Use bundled SQLite for consistent session snapshots in images without Python 3.7, preserving observations and cached descendant usage. Upload the Gradle mirror config to its absolute path so Docker Java task setup succeeds. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
giuliolovisotto
changed the title
This was referenced Oct 7, 2026
Move the snapshot and Gradle upload changes into a separate PR so this branch only changes cached-token accounting. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
giuliolovisotto
changed the title
giuliolovisotto
enabled auto-merge
October 7, 2026 10:43
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/37614029255 |
wdykas
pushed a commit
to wdykas/Gym
that referenced
this pull request
Oct 8, 2026
Adds health expectations to the conformance checker introduced by NVIDIA-NeMo#4041. Rebased onto main after NVIDIA-NeMo#4041 merged; this PR contains only the health-check addition. A harness can satisfy artifact checks while Gym health still lacks required evidence or finds incorrect aggregate usage. Add P0 `HealthCheck` objects generated from Gym's rollout health registry. The conformance runner calls the public health engine once on saved rollout and failure artifacts, then compares each individual health result with the scenario's expectation. Scenarios default to `healthy` and manually declare exceptions. Deliberately omitted usage and injected HTTP errors expect missing-token findings and unobserved token reconciliation; terminal HTTP errors also expect an ended-on-failed-call finding. Missing health results fail. New registered rollout checks default to healthy; unknown scenario override IDs are configuration errors. Explicit step scope remains respected, and task-level checks requiring repeats remain outside this suite. Results appear in a separate Health group in the existing scenario tables. A pass means the expected outcome occurred, including expected unhealthy or unobserved outcomes. Gym's health-check logic and production harness behavior are unchanged. Tracking: [FEA-402](https://linear.app/nvidia/issue/FEA-402). Validation after rebasing onto main (`cc2046c84`), 8 October 2026: - 395 checker, runner and rollout-health tests passed; 2 skipped (Python 3.13.14). - `pre-commit run --all-files` passed. - Fern check: 0 errors, 1 warning. - Preserved main’s moved reader helpers and shared evidence/witness fixture. Updated the step-scope regression test: invocation-owned token reconciliation can pass without turns, and a real aggregate mismatch still fails when step checks are excluded. Original pre-rebase runtime validation: - 293 checker/runner tests passed; 2 skipped. - `pre-commit run --all-files` passed. - Fern check: 0 errors, 1 warning. - Seven fresh scripted-model Pi scenarios completed. Four match every health expectation; retry and terminal-error scenarios expose existing ownership gaps. - Seven saved sandboxed OpenCode scenarios from NVIDIA-NeMo#4106 reinspected using their compatible native Gym health version: all health expectations match. These artifacts use a newer trajectory schema than this branch's base, so the matching health reader is required. No new harness/model execution was performed for this conflict resolution; full server CI was not run locally. The changes resolve imports and test-fixture compatibility without changing the health expectation implementation. --------- Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
yaoyu-33
pushed a commit
to yaoyu-33/Gym
that referenced
this pull request
Oct 9, 2026
…#4172) OpenCode observation collection currently runs Python's SQLite backup API inside each task image. Images without Python, or with Python older than 3.7, cannot produce the snapshot: native evidence becomes unobserved and child-session usage absent from the parent export is lost. Use OpenCode's bundled SQLite through `opencode db "VACUUM INTO ..."` to create a consistent standalone snapshot, including committed WAL rows. Preserve session-selection environment, timeout and failure reporting; escape the destination as a SQL literal and shell argument. Update snapshot tests to exercise the real SQL query against a live WAL database and preserve failure coverage. Also upload the SWE-bench Java Gradle mirror configuration to `/root/.gradle/init.d/maven_central_mirror.gradle`. The Docker upload path is not expanded by a shell, so `~/.gradle/...` failed during Druid/Lucene setup. **This path fix is validated on Docker only; no Slurm validation is claimed.** Split from NVIDIA-NeMo#4106 so the cached-token sums can be reviewed independently. This PR targets `main`, does not depend on NVIDIA-NeMo#4106, and does not change token conversion formulas. No separate issue is needed for these reproduced failures from the NVIDIA-NeMo#3764 follow-up evaluation. ## Validation At `1e28f88d8`, using `gym env test` with `+should_validate_data=true ++skip_venv_if_present=true`: - Sandboxed OpenCode agent: **50 passed**, including snapshots with committed WAL rows, database-name variants, child/grandchild ownership, and failed snapshot commands with observations enabled/disabled. - SWE-bench resources: **28 passed**; example-data validation passed. - `pre-commit run --all-files` and `git diff --check`: passed. Previously, the same runtime snapshot and Gradle changes were tested together with the cached-token fix at `266ec1154056cf31a54950f6def5e193cebfebdc`: - Stock OpenCode 1.17.11 snapshot fixtures passed with Python absent and Python 3.5.6, including committed WAL data, integrity checks and quoted/metacharacter destination paths. These fixtures made no model calls. - Seven targeted SWE-bench reruns completed: **6 healthy / 1 unhealthy / 0 unobserved**, compared with six unobserved in the initial batch. All 322 calls reconciled in the combined implementation. The remaining unhealthy result was Caddy hitting a generation length limit, not missing evidence. - Druid and Lucene completed setup with the committed absolute upload destination. The live reruns used stock OpenCode 1.17.11 and `azure/openai/gpt-4.1`, capture/observability enabled, Docker on macOS arm64, and the supported file-watcher-disable setting for a separate shutdown hang. Selected CPU and step-budget settings changed between original and follow-up runs, so this is not a controlled score comparison. These are retained combined-change rollout results, not new standalone-branch rollouts; accurate cached-token totals additionally require NVIDIA-NeMo#4106. ## Compatibility and benchmark impact No configuration migration. Task images no longer need Python for this snapshot operation. Successful collection restores native observations and descendant usage where collection previously failed; Java Docker tasks can complete Gradle configuration upload. Model sampling and verifier logic are unchanged. The native SQLite command was validated with OpenCode 1.17.11. Slurm confirmation of the Java setup fix remains outstanding. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
This branch was previously 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
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.
OpenCode reports uncached input, cache reads, and cache writes separately. The sandboxed adapter omitted both cache categories from
input_tokens; the local adapter omitted cache writes. For example, 122 uncached input tokens plus 5,760 cached reads must report 5,882 input tokens, not 122.Both adapters now sum uncached input + cache reads + cache writes exactly once. Cached-read details and output/reasoning accounting from #3764 retain their existing semantics. Regressions cover cache reads, writes, and absent/null cache counters. This is a direct follow-up to #3764; a separate issue is unnecessary for this bounded correction.
OpenCode 1.17.11 explicitly subtracts both cache reads and cache writes from its stored
inputcount (upstream implementation). Cache writes are prompt tokens used to populate the cache; adding them back reconstructs prompt size, rather than counting an extra copy or applying a billing multiplier. Gym capture already normalizes to full prompt size, and the health check compares that total with the adapter total.This PR contains only token conversion and its tests. Snapshot collection and the Gradle upload correction are tracked separately in #4172.
Validation
At
ba60d5f48, usinggym env testwith+should_validate_data=true ++skip_venv_if_present=true:pre-commit run --all-filesandgit diff --check: passed.The complete tracked tree is identical to the original cached-token commit
afe8826306eae36844045cebac6ce9fa114f65f6, which was tested on 10 SWE-bench Verified, 10 Multilingual, and 10 Pro tasks (one repeat each). Across 1,297 captured calls, independent comparison found zero input/output/cache mismatches in 1,077 completed calls with canonical ownership, including 62 child calls; the previous conversion undercounted 1,059 of those calls. Cache writes were zero in live runs and are covered by regression tests.That original batch produced 23 healthy / 1 unhealthy / 6 unobserved rollouts. The six unobserved outcomes were snapshot collection failures addressed by the separate PR. The unhealthy run had a native timeout and missing final-stream usage. The later combined-fix reruns are not evidence that this cache-only PR resolves those failures.
Runs used stock OpenCode 1.17.11,
azure/openai/gpt-4.1, model-call capture and observability, amd64 Docker on macOS arm64, a 1,800-second native timeout, and the supported file-watcher-disable setting for a separate shutdown hang. Initial Java tasks used an isolated Gradle-path overlay. This batch therefore validates token reconciliation, not the Java setup correction in this PR.Compatibility
No configuration migration. Reported input totals increase by previously omitted cached tokens. No change to model sampling or verification.