Fix run_in_loop.py: re-encode UTF-16LE to UTF-8 and pass reasoning_effort to the client - #230
Merged
Merged
Conversation
…TF-8 The file has been committed as UTF-16LE with a BOM since 0f66dc8 (2026-05-09). Python cannot import such a source file: SyntaxError: source code string cannot contain null bytes so `from run_in_loop import run_one` failed, which took down scripts/validate_driver.py (it reports a setup error) and the agent-in-loop path of experiments/swebench_lite/run.py. Nothing in the test suite imports the module, so CI never noticed. The content is unchanged; only the encoding is. Line endings are preserved. Adds tests/test_source_encoding.py, which walks src/, scripts/, experiments/, tests/ and benchmarks/ and asserts every .py file has no UTF-16 BOM, no NUL bytes, and parses. A scan of all tracked files found run_in_loop.py to be the only UTF-16 file.
🤖 Godspeed Review
SecurityNo secrets detected in changed files. |
run_in_loop._run_one_async built its LLMClient from thinking_budget only and dropped settings.reasoning_effort. For a Qwen3.5+/3.8 model the client then sends no chat-template kwargs (_qwen_template_kwargs returns None), so the server's template default (xhigh thinking) applies whatever the settings say: the in-loop benchmark silently ran a different configuration than requested. Adds tests/test_run_in_loop_wiring.py: it points the global settings.yaml at a temp dir with `reasoning_effort: medium`, runs _run_one_async with a capturing LLMClient subclass and a stub agent loop, and asserts the client received reasoning_effort="medium". It fails without the one-line change. Note for callers: GodspeedSettings() applies YAML but not GODSPEED_* env vars (only load_settings() does), so benchmark configuration for this runner has to come from settings.yaml.
t-timms
changed the title
🤖 Godspeed Review
SecurityNo secrets detected in changed files. |
🤖 Godspeed Review
SecurityNo secrets detected in changed files. |
🤖 Godspeed Review
SecurityNo secrets detected in changed files. |
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.
Why
experiments/swebench_lite/run_in_loop.pyhad two independent problems:Unimportable. It is committed as UTF-16LE with a BOM (introduced in 0f66dc8, 2026-05-09; UTF-8 in a92656b and earlier). Python cannot import it:
scripts/validate_driver.pyand the agent-in-loop path ofexperiments/swebench_lite/run.pybothimport run_in_loop, so both have been broken since. No test imports the module, so CI stayed green.Drops the configured reasoning effort. It builds
LLMClientfromthinking_budgetonly. For a Qwen3.5+/3.8 model the client then sends no chat-template kwargs, so the server's template default (xhighthinking) applies regardless ofreasoning_effortin settings. The benchmark silently runs a different configuration than the one requested.Changes
run_in_loop.pyas UTF-8 (content and line endings unchanged).reasoning_effort=settings.reasoning_efforttoLLMClient.tests/test_source_encoding.py: every.pyundersrc/,scripts/,experiments/,tests/,benchmarks/(exceptbenchmarks/fixtures/, which holds intentionally broken code) has no UTF-16 BOM, no NUL bytes, and parses.tests/test_run_in_loop_wiring.py: withreasoning_effort: mediumin the global settings.yaml, the client built by_run_one_asyncreceivesreasoning_effort="medium".Verification
run_in_loop.pywas the only one.run_in_loop.py); with the fix 465 passed.import run_in_loopnow succeeds.ruff check/ruff format --checkclean.Note
GodspeedSettings()applies YAML but notGODSPEED_*env vars (onlyload_settings()does), and this runner uses the constructor, so its configuration has to come fromsettings.yaml.Not verified