Skip to content

feat(viewer): dark theme and syntax highlighting for tool content - #1098

Open
Benjamin-eecs wants to merge 2 commits into
benchflow-ai:mainfrom
Benjamin-eecs:feat-viewer-dark-mode-highlight
Open

feat(viewer): dark theme and syntax highlighting for tool content#1098
Benjamin-eecs wants to merge 2 commits into
benchflow-ai:mainfrom
Benjamin-eecs:feat-viewer-dark-mode-highlight

Conversation

@Benjamin-eecs

@Benjamin-eecs Benjamin-eecs commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a dark theme and syntax highlighting to the interactive trajectory viewer.

  • theme.css: the existing light tokens stay the unconditional default; a html[data-theme="dark"] block overrides the same tokens (canonical, viewer aliases, tool-kind accents) and sets color-scheme: dark. A page without the attribute renders exactly as before.
  • template.html: a head script applies the stored choice (localStorage key bf-theme) or the system preference before the stylesheet, so there is no flash; a toggle sits in the wordmark.
  • theme.js (new): the toggle, plus BF.highlight, which colors a tool content block only when its language is known: a markdown fence tag, a diff signature, the first block of an execute card (bash, with python - <<EOF heredoc bodies as Python), or the file extension in a read/edit title. Nothing is auto-detected, so plain logs stay plain. Blocks carrying redaction markers are left untouched.
  • viewer.css: token color rules driven by --hl-* theme tokens; terminal blocks keep the dark palette in both themes.
  • assets/vendor/highlight.min.js: highlight.js 11.9.0 common bundle (BSD-3, license file alongside), inlined by render.py like the other assets, so the page stays self-contained.

Motivation and Context

Closes #1097. Reviewing long trajectories on the light page is tiring, and a multi-hundred-line heredoc inside an execute card reads as one grey block. Both features are running on the FrontierPhysics deployment of this viewer.

How Has This Been Tested?

  • tests/trajectories/test_viewer_theme.py: light-first token order, dark override contents, head script before the stylesheet, toggle present, vendored highlighter inlined before the app scripts, no CDN reference, rendered shell carries both themes.
  • pytest tests/trajectories: 293 passed, 29 skipped (playwright absent).
  • Manual: browse pages of a 540-rollout FrontierPhysics run in both themes, execute/edit/read cards of codex-acp and claude-agent-acp rollouts.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Devin Review

Copilot AI lite review requested due to automatic review settings September 4, 2026 17:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

_PAYLOAD_PLACEHOLDER = "__BENCHFLOW_PAYLOAD__"
_TITLE_PLACEHOLDER = "__BENCHFLOW_TITLE__"
_SCRIPT_ASSETS = ("viewer.js", "detail.js", "catalog.js", "boot.js")
_SCRIPT_ASSETS = ("theme.js", "viewer.js", "detail.js", "catalog.js", "boot.js")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Every trajectory viewer fails during startup

The combined script reaches BF.theme before const BF initializes. A ReferenceError aborts startup, leaving every trajectory and catalog unrendered.

Suggested change
_SCRIPT_ASSETS = ("theme.js", "viewer.js", "detail.js", "catalog.js", "boot.js")
_SCRIPT_ASSETS = ("viewer.js", "theme.js", "detail.js", "catalog.js", "boot.js")
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1cd670a: theme.js now loads after viewer.js and test_viewer_theme asserts the order.

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.

[Feature] viewer: dark theme and syntax highlighting for tool content

2 participants