config: web_fetch back on by default; description documents the strip + cap#410
Merged
Conversation
web_fetch (and memory_fetch, gated on it) were turned OFF by default in #314's lean-edit shape. But the tool's value over shell curl is exactly the HTML->readable-text stripping (tags removed, entities decoded) capped at max_chars (default 50000) -- curl returns raw HTML the model then has to hand-scrape. Turn it back on, and make the description say plainly what the model gets: readable plain text, the char cap, that save_to writes the uncapped full body to the workspace, and that it does NOT run JavaScript (a JS-rendered SPA returns little -- fetch a raw/API/README URL instead). - TConfig.Create default WebFetchEnabled := True. - ToJSON now emits web_fetch_enabled only on the explicit-OFF (mirrors the other default-on fields) so opting out round-trips and a fresh config stays tidy. - stock profile mirrors the new default (web_fetch_enabled: true). - Tests updated: default is True; the round-trip tests now exercise the opt-OUT direction (set false -> emitted -> loads false). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGQKz579j1ZnDRwmr6h1V6
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 530e6003d7
ℹ️ 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".
| PromptCache.TTL := '1h'; { 1h cache hits well across back-to-back runs (bench/swe/results/ablation.md). 5m was the historical default; 1h is one of the six zero-prompt-cost behavioral toggles the bench identified as a free upgrade. } | ||
| VaultToolsEnabled := False; { off by default per the bench-grounded "stock = lean-edit shape" verdict (bench/swe/README.md). Vault entries are never called across the bench's 45+ cells -- the model has them as training data. Onboarding asks (default Y for operators who DO use the vault). } | ||
| WebFetchEnabled := False; { off by default for the same reason as VaultToolsEnabled. Also drops memory_fetch (RegisterMemoryFetchTool is gated on EnableWebFetch in NewBuiltinRegistry -- see comment there). Onboarding asks. } | ||
| WebFetchEnabled := True; { on by default: the tool clearly documents that it returns readable plain text (HTML tags stripped, entities decoded) capped at max_chars (default 50000, save_to bypasses), so the model knows what it gets -- and a "read this URL" task shouldn't have to fall back to hand-rolled shell curl + HTML scraping. Also enables memory_fetch (RegisterMemoryFetchTool is gated on EnableWebFetch in NewBuiltinRegistry). Onboarding asks; operators wanting no outbound HTTP from the agent set web_fetch_enabled: false. } |
There was a problem hiding this comment.
When an operator selects --profile low-token, LoadConfig starts from TConfig.Create and then applies Profile_LowToken, which does not set web_fetch_enabled; this new default therefore makes the low-token profile register both web_fetch and the gated memory_fetch schemas on every turn. That regresses the profile's token-minimizing behavior and makes max-build's explicit web_fetch_enabled:true override effectively redundant; add web_fetch_enabled:false to Profile_LowToken if the default-on change is meant for stock/no-profile runs only.
Useful? React with 👍 / 👎.
…w fix) With web_fetch now on by default, --profile low-token (which starts from TConfig.Create then layers its JSON but never set web_fetch_enabled) inherited the on-default -- registering web_fetch + the gated memory_fetch schemas every turn, regressing the profile's token-minimizing intent. Add "web_fetch_enabled":false to Profile_LowToken. max-build inherits low-token, so its explicit web_fetch_enabled:true now meaningfully overrides this (no longer redundant). Baseline and security already pin it off; stock/no-profile keep the new on-default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGQKz579j1ZnDRwmr6h1V6
Reverses the previous low-token opt-out: web_fetch is on by default across
the general/capability profiles now (stock, low-token, max-build, all-on).
The only profiles that keep it off are the two whose explicit purpose is a
capability lockdown: `security` ("no outbound HTTP" -- keeping it on would
break that guarantee) and `baseline` ("everything off" A/B-test floor).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGQKz579j1ZnDRwmr6h1V6
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
2 participants
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
web_fetch(andmemory_fetch, gated on it) were turned off by default in #314's lean-edit shape — the reasoning being it looked redundant withshell_exec+curl. But the tool's actual value over curl is the HTML→readable-text stripping (tags removed, entities decoded) capped atmax_chars; curl returns raw HTML the model then has to hand-scrape (exactly the throwaway-parse_html.pyloop seen in a real failing run). So: turn it back on, and make the description state plainly what the model gets.Changes
TConfig.CreatedefaultWebFetchEnabled := True.ToJSONnow emitsweb_fetch_enabledonly on the explicit-OFF (mirrors the other default-on fields), so opting out round-trips and a fresh config stays tidy.stockprofile mirrors the new default (web_fetch_enabled: true).True; the round-trip tests exercise the opt-OUT direction (set false → emitted → loads false).Test
test-component-config,test-config-profile,test-loop-shaping-defaultsgreen; full build clean. Confirmed a fresh config reports web_fetch at its default (absent fromToJSON= default-on).🤖 Generated with Claude Code
https://claude.ai/code/session_01LGQKz579j1ZnDRwmr6h1V6
Generated by Claude Code