Skip to content

fix: preload generated trace viewer content#161

Open
liaohch3 wants to merge 1 commit into
mainfrom
fix/static-preload-trace-viewer
Open

fix: preload generated trace viewer content#161
liaohch3 wants to merge 1 commit into
mainfrom
fix/static-preload-trace-viewer

Conversation

@liaohch3

Copy link
Copy Markdown
Owner

Summary

  • prerender the generated trace viewer sidebar, stats, path filters, and first detail pane into exported HTML
  • keep the existing JavaScript viewer behavior by clearing the preloaded DOM before client-side rendering
  • add no-JS Playwright coverage for generated trace HTML

Evidence

No-JS static preload viewer

Test plan

  • uv run ruff check .
  • uv run ruff format --check .
  • uv run pytest tests/ -x --timeout=60
  • uv run python scripts/check_screenshots.py .agents/evidence/pr/static-preload-viewer/desktop-nojs.png
  • uv run python scripts/verify_screenshots.py /home/liaohch3/src/github.com/liaohch3/codex-acme-sdk-python-private/eval/results/traces/python-sdk-smoke/2026-05-12/trace_103102_static_preload.html

Render an initial trace sidebar and detail pane into generated HTML so file previews that do not run JavaScript still show the captured trace. Keep normal browser behavior by clearing the preloaded DOM before the client renderer hydrates.

Co-authored-by: Talon <talon@users.noreply.github.com>
@YoungCan-Wang

YoungCan-Wang commented May 30, 2026

Copy link
Copy Markdown
Collaborator

@liaohch3
经过对 PR #161fix: preload trace viewer content)补丁代码的审查,发现以下几个问题和改进点:

1. 🔴 逻辑运算符优先级 Bug(_is_context_only 函数)

claude_tap/viewer.py 中,判定是否仅有请求上下文(无独立响应内容)的函数存在运算符优先级缺陷:

def _is_context_only(record: dict, messages: list[dict], response_output: dict | None) -> bool:
    body = _request_body(record)
    if not messages or not isinstance(body.get("input"), list):
        return False
    return (
        "/codex/responses" in _record_path(record)
        or "/v1/responses" in _record_path(record)
        and response_output is None
    )
  • 原因:Python 中 and 的优先级高于 or。因此该表达式会被解析为:
    ("/codex/responses" in path) or ("/v1/responses" in path and response_output is None)
  • 后果:对于所有请求路径包含 /codex/responses 的 Codex 记录,即使 response_output 存在值,该函数也会错误地返回 True。这会导致在静态预加载视图中,即使有正常的响应,消息区块的标题也会被错误地显示为 "Context"(上下文)而不是 "Messages"(消息)。
  • 修复建议:应显式加括号:
    return (
        ("/codex/responses" in _record_path(record) or "/v1/responses" in _record_path(record))
        and response_output is None
    )

2. 🟡 潜在的未捕获类型/值异常(鲁棒性问题)

在预加载内容渲染和统计数据计算中,存在多处直接调用 int(...) 强转的地方,未像 duration_ms 一样用 try-except 保护:

  • status = int(response.get("status") or 0)
  • input_tokens = int(usage.get("input_tokens") or 0)
  • output_tokens = int(usage.get("output_tokens") or 0)
  • cache_read = int(usage.get("cache_read_input_tokens") or 0)
  • cache_create = int(usage.get("cache_creation_input_tokens") or 0)
  • 后果:如果上游返回的 status 或 token 数量由于非正常情况(如出错、Mock 数据、类型不匹配等)含有非整型字符(例如 """unknown"),int() 将抛出 ValueError/TypeError 异常,从而导致整个 trace viewer 的 HTML 生成过程崩溃。
  • 修复建议:定义一个类似于 _safe_int(val) -> int 的辅助函数来统一处理转换,确保在发生异常时优雅地回退到 0

3. 🟢 轻微的客户端水合布局闪烁(CSS 闪烁问题)

viewer.html 模板中,主要的容器布局定义了:
style="display:none;display:flex;flex-direction:column"

  • 说明:在 CSS 中,后声明的 display:flex 会覆盖前面的 display:none。这使得在 JS 加载前容器是 flex 布局且可见的,方便静态预加载内容的显示。但如果此时用户加载的是一个空白的或者正在等待连接的 live 页面,在 JS 执行后会由 JS 重新计算并将其置为 display:none。这可能会在首屏渲染时造成短暂的布局结构跳变/闪烁,虽然不影响功能,但属于一个微小的视觉体验缺陷。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants