Repository navigation
fix(agent): the capability table reads the picture switch the loop reads - #463
Conversation
image_search arrived with a second switch beside its credential: tools.web.search.images decides whether the tool is registered at all (wiring.py), so a lane that never places a picture keeps the tool face it had. The capability table was not told. is_configured answers on the vendor key alone, so with a Serper key and the switch left off the table ticked a capability the agent does not hold, and two tests that drive a real AgentLoop caught the disagreement the module exists to prevent. The switch belongs on is_disabled rather than is_configured. Its own docstring already names the reason: a switched-off tool usually has its credential set, and calling it unconfigured sends the deployer to set a key that is already there. tools.disabledTools and this flag are the same kind of answer, so is_offered needs no change. The test configs that mean "everything supplied" now turn the switch on, the way core/runtime.py passes it from the config, and a new case pins the combination that was wrong: a key present, pictures off, nothing offered, and the table saying off rather than unconfigured. Two more pieces of the same arrival: - ImageSearchTool copied WebSearchTool's key resolution without its two annotations, so the untyped lambda wiring.py hands it left the api_key property returning `~AlwaysFalsy | str` against a declared str. The pinned type checker reports it; the annotations are the fix. - WebSearchConfig gained the images field, so the writer's validated subtree carries it and two test_config_update_tools expectations that pin the written section by value went stale. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: raven doctor reports the image-search switch under the wrong configuration path.
I reviewed github/main...HEAD, the capability consumers and AgentLoop/runtime wiring, the originating history, backward compatibility, test intent (including whether the parity fixtures were weakened), and the repository rules in AGENTS.md/CLAUDE.md plus the Runtime context map. The core predicate now matches registration, and the fixture changes preserve the intended parity check, but the doctor renderer still assumes every disabled state comes from tools.disabledTools; the inline finding is therefore user-visible and blocking.
Verification: uv run --frozen --python 3.12 --extra dev pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py tests/test_agent_loop_web_tools.py tests/test_cli_doctor_commands.py -q (181 passed); targeted ty check and Ruff check/format passed; commit-message and source-language checks passed. GitHub's lint, repository, wheel, UI, TUI, and bridge checks were green when reviewed; unit shards were still running.
The off state now has two sources, and the renderer had one hardcoded. A deployment holding a Serper key with tools.web.search.images at its default printed "image_search: x / switched off in tools.disabledTools", a list that need not contain the tool and whose editing cannot turn it on. The predicate was right and the repair it suggested was wrong, which is the failure these rows exist to prevent. disabled_by returns the setting rather than a flag, is_disabled becomes bool() of it, and the capability row carries it to both renderings and to the JSON payload. Where both switches are shut the registration gate is named first: taking image_search out of disabledTools while pictures are off restarts into the same missing tool. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new revision fixes the outstanding doctor-reporting defect: the capability layer now preserves which setting disabled a tool, the text and JSON surfaces carry that source, and the precedence when both switches are off identifies the registration gate that must be enabled first. I reviewed the delta and full github/main...HEAD diff, affected capability/doctor callers, originating history, backward compatibility of the additive JSON field, test intent and fixture strength, and the repository/runtime rules previously applied. I found no new plain error.
Verification: uv run --frozen --python 3.12 --extra dev pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py tests/test_agent_loop_web_tools.py tests/test_cli_doctor_commands.py -q (185 passed); targeted ty check, Ruff check/format, commit-message validation, and source-language validation all passed. The unrelated idle-ceiling observation is outside this diff and does not block this change. My prior thread is resolved.
…ads (#463) ## Summary `main` is red on three jobs (`python lint`, `unit 2/4`, `unit 3/4`), and has been since #451 merged. All of it is fallout from one arrival: `image_search` came with a second switch beside its credential. `tools.web.search.images` decides whether the tool is registered at all (`raven/agent/loop/wiring.py`), so a lane that never places a picture keeps the tool face it had. The capability table was not told. `is_configured` answers on the vendor key alone, so with a Serper key and the switch left off the table ticked a capability the agent does not hold, and the two tests that drive a real `AgentLoop` caught exactly the disagreement that module exists to prevent. The switch belongs on `is_disabled`, not `is_configured`. That function's own docstring already names the reason: a switched-off tool usually has its credential set, and calling it unconfigured sends the deployer to set a key that is already there. `tools.disabledTools` and this flag are the same kind of answer, so `is_offered` needs no change and the doctor row reads "off" rather than "missing a key". The test configs that mean "everything supplied" now turn the switch on, the way `raven/core/runtime.py` passes it from the config, and a new case pins the combination that was wrong: key present, pictures off, nothing offered, table saying off. Two more pieces of the same arrival: - `ImageSearchTool` copied `WebSearchTool`'s key resolution without its two attribute annotations, so the untyped lambda `wiring.py` hands it left the `api_key` property returning `~AlwaysFalsy | str` against a declared `str`. The type checker pinned in #416 reports it; the annotations are the fix. - `WebSearchConfig` gained the `images` field, so the writer's validated subtree now carries it and two `test_config_update_tools` expectations that pin the written section by value went stale. Not changed here: `get_web_search` still reports `{provider, api_key, max_results}` and does not surface `images`, though `set_web_search` can write it. That is a gap in the read side of the config API, not a CI failure, and it wants its own change. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Run on Windows 11 in a clean worktree cut from `main` (`5a29ea22`): - `uv run pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py -q`: 71 passed. All four tests failing on `main` now pass, plus the new case. - `uv run --frozen --python 3.12 --all-extras ty check raven evolver agents plugins-dist scripts`: the `web.py:516` diagnostic is gone. The diagnostics that remain locally are unresolved imports for optional packages absent from this machine's environment (`boxlite`, `nio`); CI reported exactly one diagnostic before this change, the one fixed here. - `uv run --frozen --python 3.12 --extra dev ruff check raven tests`: all checks passed. - `ruff format --check` on the four changed files: already formatted. - `uv run pytest tests/test_cli_doctor_commands.py tests/test_rpc_console.py -q`, which consume `is_disabled`: the failing-test set is byte-identical before and after this change (8 pre-existing Windows-only failures around file modes and an absent browser package, none of them in the set CI reports). ## Risk `is_disabled` gains a second reason to answer true, so a deployment with a search key and `tools.web.search.images` off now reads as "switched off" in `raven doctor` and the console capability report, where it previously read as offered. That is the state the loop was already in; only the description changes. Nothing about registration, gating, or the tool's runtime behaviour moves. The other two changes are an annotation and two test expectations, with no runtime effect. Rollback is reverting this commit; it touches nothing that other work builds on. ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
mainis red on three jobs (python lint,unit 2/4,unit 3/4), and has beensince #451 merged. All of it is fallout from one arrival:
image_searchcame with asecond switch beside its credential.
tools.web.search.imagesdecides whether the tool is registered at all(
raven/agent/loop/wiring.py), so a lane that never places a picture keeps the toolface it had. The capability table was not told.
is_configuredanswers on the vendorkey alone, so with a Serper key and the switch left off the table ticked a capability
the agent does not hold, and the two tests that drive a real
AgentLoopcaught exactlythe disagreement that module exists to prevent.
The switch belongs on
is_disabled, notis_configured. That function's own docstringalready names the reason: a switched-off tool usually has its credential set, and
calling it unconfigured sends the deployer to set a key that is already there.
tools.disabledToolsand this flag are the same kind of answer, sois_offeredneedsno change and the doctor row reads "off" rather than "missing a key".
The test configs that mean "everything supplied" now turn the switch on, the way
raven/core/runtime.pypasses it from the config, and a new case pins the combinationthat was wrong: key present, pictures off, nothing offered, table saying off.
Two more pieces of the same arrival:
ImageSearchToolcopiedWebSearchTool's key resolution without its two attributeannotations, so the untyped lambda
wiring.pyhands it left theapi_keypropertyreturning
~AlwaysFalsy | stragainst a declaredstr. The type checker pinned inchore(*): gate call correctness with a pinned type checker #416 reports it; the annotations are the fix.
WebSearchConfiggained theimagesfield, so the writer's validated subtree nowcarries it and two
test_config_update_toolsexpectations that pin the writtensection by value went stale.
Not changed here:
get_web_searchstill reports{provider, api_key, max_results}anddoes not surface
images, thoughset_web_searchcan write it. That is a gap in theread side of the config API, not a CI failure, and it wants its own change.
Type
Verification
Run on Windows 11 in a clean worktree cut from
main(5a29ea22):uv run pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py -q:71 passed. All four tests failing on
mainnow pass, plus the new case.uv run --frozen --python 3.12 --all-extras ty check raven evolver agents plugins-dist scripts:the
web.py:516diagnostic is gone. The diagnostics that remain locally areunresolved imports for optional packages absent from this machine's environment
(
boxlite,nio); CI reported exactly one diagnostic before this change, the onefixed here.
uv run --frozen --python 3.12 --extra dev ruff check raven tests: all checks passed.ruff format --checkon the four changed files: already formatted.uv run pytest tests/test_cli_doctor_commands.py tests/test_rpc_console.py -q, whichconsume
is_disabled: the failing-test set is byte-identical before and after thischange (8 pre-existing Windows-only failures around file modes and an absent browser
package, none of them in the set CI reports).
Risk
is_disabledgains a second reason to answer true, so a deployment with a search keyand
tools.web.search.imagesoff now reads as "switched off" inraven doctorand theconsole capability report, where it previously read as offered. That is the state the
loop was already in; only the description changes. Nothing about registration, gating,
or the tool's runtime behaviour moves.
The other two changes are an annotation and two test expectations, with no runtime
effect.
Rollback is reverting this commit; it touches nothing that other work builds on.
Related Issues
N/A