Python: Add WebSearchDisplayObserver to harness console - #6572
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Python harness-console observer to surface web search activity (queries, open-page, find-in-page) in the streaming console output, bringing parity with the existing .NET harness behavior.
Changes:
- Introduces
WebSearchDisplayObserverto render completedsearch_tool_resultitems forweb_searchin a tree-style console format. - Registers the observer in the default and planning observer pipelines.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| python/samples/02-agents/harness/console/observers/web_search_display.py | New observer that formats and prints completed web-search actions to the console. |
| python/samples/02-agents/harness/console/observers/init.py | Wires the new observer into the standard observer lists and exports it. |
…bserver Apply rich.markup.escape() to all user/tool-provided strings (queries, URLs, titles, patterns) before interpolation into Rich-markup-enabled output. This prevents characters like '['/']' from being interpreted as Rich markup tags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 91%
✓ Correctness
The code logic, data access patterns, and observer registration are all correct. The only issue is the missing Rich markup escaping of dynamic content (queries, URLs, titles), which is already comprehensively covered by the existing unresolved review comments. All other observers in this codebase (reasoning_display, planning_output, text_output) use
from rich.markup import escapebefore passing dynamic text toux.append_info_line(). No new issues found beyond the existing comments.
✗ Security Reliability
The PR adds a WebSearchDisplayObserver that renders dynamic, untrusted content (search queries, URLs, source titles) into a Rich-markup-enabled panel without escaping. Other observers in the same package (TextOutputObserver, ReasoningDisplayObserver, PlanningOutputObserver) all use
from rich.markup import escapebefore passing dynamic text toux.append_info_lineorux.write_text. The new observer omits this escaping at every interpolation point. If a search query or URL contains[or]characters (common in URLs with query parameters), Rich will attempt to parse them as markup tags, causing rendering corruption or exceptions. The existing unresolved review comments correctly identify all affected lines; I concur with those findings and have no additional issues to report.
✓ Test Coverage
The new WebSearchDisplayObserver adds non-trivial formatting logic (multiple action types, tree-view rendering, truncation, source formatting) but includes no unit tests. While this is consistent with the existing pattern — no other observer in this samples package has tests — the observer has enough branching logic and edge-case handling that lightweight unit tests would catch regressions cheaply. The helper functions (_truncate, _format_source) and the various action-type code paths are easily testable in isolation without complex mocking.
✓ Failure Modes
The only concrete failure mode in this diff is the Rich markup injection issue already covered by the 5 existing unresolved review comments. The scroll panel uses markup=True (scroll_panel.py:39), and the _format_text method wraps content in color tags (scroll_panel.py:118-119), meaning any '['/']' in dynamic strings (queries, URLs, titles, patterns) will be interpreted as Rich markup — causing either rendering corruption or a MarkupError crash that propagates up the observer pipeline. Other observers (reasoning_display.py:9,45; text_output.py:9,41) correctly import and use rich.markup.escape for dynamic content. No additional failure modes beyond the existing comments were found.
✓ Design Approach
The new observer covers the Foundry/OpenAI search-result shape, but it is registered in the generic harness observer lists while only listening for
search_tool_result. That means Anthropic-backed web search sessions still produce no web-search UI, so the feature is only partially implemented across the harness.
Flagged Issues
- All dynamic content (queries, URLs, source titles/patterns) is interpolated into Rich-markup strings without escaping. The RichLog uses markup=True and _format_text wraps content in [color]...[/color] tags, so any brackets in tool-provided data will be misinterpreted as markup, causing rendering corruption or MarkupError crashes. All other observers in this package consistently use
from rich.markup import escapefor dynamic content.
Automated review by westey-m's agents
|
Flagged issue All dynamic content (queries, URLs, source titles/patterns) is interpolated into Rich-markup strings without escaping. The RichLog uses markup=True and _format_text wraps content in [color]...[/color] tags, so any brackets in tool-provided data will be misinterpreted as markup, causing rendering corruption or MarkupError crashes. All other observers in this package consistently use Source: automated DevFlow PR review |
Motivation & Context
The Python harness console did not display web search activity (search queries, page opens, find-in-page actions) when using models with web search tools. The .NET harness console already has a
WebSearchDisplayObserverthat shows this activity, but the Python port was missing this observer. Users running theharness_research.pysample saw no indication of web search tool calls in the output stream.Description & Review Guide
What are the major changes?
WebSearchDisplayObserverinpython/samples/02-agents/harness/console/observers/web_search_display.pybuild_default_observers()andbuild_observers_with_planning()in__init__.pyWhat is the impact of these changes?
search_tool_resultcontent (emitted onresponse.output_item.done) rather thansearch_tool_call(emitted onresponse.output_item.added), because the action details are only fully populated when the search completes.What do you want reviewers to focus on?
result["action"]["type"],action["queries"], etc.) correctly matches the serialized OpenAI SDKActionSearch/ActionOpenPage/ActionFindmodels.search_tool_callfor an early "searching..." indicator.Related Issue
No linked issue — this is a feature addition to the Python harness console port (related to PR #6312).
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.