feat(display): declarative tool-preview schema to eliminate per-tool hardcoding (#28621)#28719
Open
xxxigm wants to merge 3 commits into
Open
feat(display): declarative tool-preview schema to eliminate per-tool hardcoding (#28621)#28719xxxigm wants to merge 3 commits into
xxxigm wants to merge 3 commits into
Conversation
…#28621) Replace the open-coded ``if tool_name == "memory" / "session_search" / ...`` chain in ``build_tool_preview`` with a small declarative registry so new tools — including plugins — can ship a one-line progress preview alongside their schema instead of waiting for a core code edit. ``register_tool_preview()`` accepts either a single template or a ``field``-dispatched mapping with ``"*"`` fallback; templates use ``str.format_map`` semantics with missing-key safety, list-arg joining, whitespace collapsing on string fields, and per-field precision truncation (``{content:.30}``). A per-tool ``truncate=N`` cap layers on top of the global ``_tool_preview_max_len``. Registry entries take priority over the legacy hardcoded chain so migration is incremental with no flag-day. Bad templates degrade to ``None`` and a warning log instead of crashing the spinner.
…ore + fact_feedback (NousResearch#28621, NousResearch#28598) Co-locates declarative preview templates with the holographic-memory plugin's tool schemas so Feishu / Telegram / CLI bubbles render ``+ "user prefers tabs"`` or ``search: "editor config"`` instead of the previous useless ``⚙️ fact_store...`` fallback. This is the proof-of-concept migration called out in NousResearch#28621's scope (plugins ship preview support without touching core display code) and directly closes the user-visible gap reported in NousResearch#28598.
…_tool_preview (NousResearch#28621) Adds 22 new unit tests pinning the registry contract — single-template rendering, field dispatch with ``"*"`` fallback, missing-key safety, list-arg joining, per-field precision truncation, per-tool ``truncate`` cap precedence over the global limit, graceful degradation on bad templates, last-write-wins re-registration, and registry-over-legacy priority — plus a smoke test for the ``fact_store`` / ``fact_feedback`` plugin registrations. Adds a "Tool Progress Previews" section to the Adding Tools developer guide so authors discover the API alongside schema authoring and the checklist nudges them to register one when relevant.
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.
What does this PR do?
Adds a declarative tool-preview schema registry to
agent/display.pyso new tools — including plugin/third-party tools — can ship a one-line progress preview alongside their schema definition, instead of editing the hardcodedif-elifchain inbuild_tool_preview()every time.Before, tools without a manual entry fell back to
⚙️ tool_name...on Feishu / Telegram bubbles and the CLI spinner. This is what prompted #28598 (fact_store/fact_feedbackshowing no useful context); #28621 generalises that observation: the if-elif chain is a structural oversight generator, and plugins have no path to preview support at all.The new API is:
Design choices:
register_tool_preview()at import time, next to their schema definition. No core code edit required.""(a partial tool call never crashes the spinner), string values are whitespace-collapsed, lists are joined with,, bad templates degrade toNone+ a warning log instead of raising.truncate=Ntakes priority over the global_tool_preview_max_len, useful for plugins that know their content is naturally short.Related Issue
Fixes #28621
Also closes #28598 (the user-visible bug that prompted this proposal —
fact_storeandfact_feedbacknow render meaningful previews).Type of Change
Changes Made
agent/display.py— addsregister_tool_preview(),_unregister_tool_preview(), the_TOOL_PREVIEW_REGISTRYmodule-level dict,_SafePreviewArgs(adictsubclass that returns""for missing keys and normalises values forstr.format_map), and_render_registered_preview().build_tool_preview()consults the registry first, applies the per-tooltruncate(falling back to the global_tool_preview_max_len), and only then falls through to the existing hardcoded chain. Also hardens the legacy path against truthy non-dictargs.plugins/memory/holographic/__init__.py— registers preview templates forfact_store(10 actions, includingadd,search,probe,reason,update,remove, plus a*fallback) andfact_feedback(helpful/unhelpful/*). The registration sits right next toFACT_STORE_SCHEMA/FACT_FEEDBACK_SCHEMAso reviewers see UX + schema together.tests/agent/test_display.py— addsTestRegisterToolPreview(15 unit tests covering single-template rendering, field dispatch with*fallback, missing-key safety, list-arg joining, per-field precision truncation, per-tool vs. global truncate precedence, whitespace collapsing,None→"", bad-template fallback with log assertion, last-write-wins re-registration, registry-over-legacy priority, and rejection of malformed registrations) plusTestFactStorePluginPreview(7 smoke tests for the holographic plugin registrations) and one extra defensive test for truthy non-dict args.website/docs/developer-guide/adding-tools.md— adds an "Optional: Tool Progress Previews" section showing the API and dispatch form, plus a checklist nudge so authors register a preview when relevant.Memory and the other complex special cases (
process,todo,send_message) are intentionally left on the legacy path in this PR — the issue's scope calls for 2–3 POC migrations with the rest deferred.memoryin particular has a"<missing old_text>"sentinel that doesn't fit the minimalfield/templates/truncateschema cleanly; if that's wanted, it should land in a follow-up PR (possibly extending the schema with an optionaldefaultsmapping).How to Test
Checklist
Code
feat(display):,feat(memory/holographic):,test(display)+docs:)scripts/run_tests.sh tests/agent/test_display.py -q(55 passed, 0 failed)Documentation & Housekeeping
website/docs/developer-guide/adding-tools.md)cli-config.yaml.example)CONTRIBUTING.md/AGENTS.mdupdates