Skip to content

feat(conversation-chip): brand mark + model name per conversation#708

Open
simonrosenberg wants to merge 4 commits into
mainfrom
feat-acp-model-chip
Open

feat(conversation-chip): brand mark + model name per conversation#708
simonrosenberg wants to merge 4 commits into
mainfrom
feat-acp-model-chip

Conversation

@simonrosenberg
Copy link
Copy Markdown
Contributor

@simonrosenberg simonrosenberg commented May 21, 2026

Why

Each conversation card should communicate which agent harness is running it and which model that harness is using. Today the ACP card shows a small "Claude Code" / "Codex" pill (harness only — no model), and OpenHands cards optionally show a plain text model name (model only — no logo). Mirror the chip we just shipped in the OpenHands web UI (OpenHands#14510) so both products converge: brand-mark icon + model text on a single inline row.

For ACP, the model name was previously unavailable to the frontend. The companion SDK PR software-agent-sdk#3347 adds ConversationInfo.current_model_name (a human-readable name resolved via the ACP available_models lookup — "Default (recommended)" / "GPT-5.5 (xhigh)" / "Opus 4.1" …) and ConversationInfo.current_model_id. This PR pins the dev stack to that SDK PR's SHA so the new fields are actually populated, and renders them in the chip.

Summary

  • New AgentBrandIcon component renders the right glyph per harness (openhands | claude-code | codex | gemini | cli-generic fallback). Path data extracted from the existing onboarding tile into src/constants/acp-brand-marks.ts for reuse.
  • ConversationCardFooter switches from the standalone pill + separate model row to a single inline chip: [brand mark] {model text}. Tooltip always carries the full harness + model so the chip is unambiguous on hover.
  • DirectConversationInfo typed for the new current_model_name / current_model_id fields. Adapter chains current_model_name → current_model_id → null for ACP conversations; OpenHands conversations continue to source from agent.llm.model (unchanged).
  • Dev pin: config/defaults.json gains agentServerGitRef: "235fed007c3c49841d34a92fb4b16d8877d6d82e". scripts/dev-safe.mjs reads it and routes the existing uvx git-ref install path so the dev stack picks up the SDK PR. Env vars still override; clear the field once the SDK ships.

Issue Number

None — follow-up to OpenHands#14510 and depends on software-agent-sdk#3347.

How to Test

npm install
npm run dev:dangerously-dockerless

config/defaults.json already pins the SDK to PR #3347's commit, so the dev stack will pull that SDK build via uvx automatically — watch for agent-server source: git (235fed007c3c49841d34a92fb4b16d8877d6d82e) in the startup log to confirm the pin is in effect.

Then:

  1. Start a Claude Code (ACP) conversation. The card chip should show the Claude burst glyph + the resolved model name (e.g. "Default (recommended)" for default config; "Opus 4.1" if you set acp_model="claude-opus-4-1").
  2. Start a Codex (ACP) conversation. Chip shows the OpenAI glyph + "GPT-5.5 (xhigh)" (or whatever the active gpt-X.Y/<tier> is).
  3. Start a Gemini CLI (ACP) conversation. Chip shows the Gemini spark + the active model name.
  4. Toggle showLlmProfiles for a native OpenHands conversation. Chip should show the OpenHands logo + the raw agent.llm.model string.
  5. Hover any chip — tooltip carries <provider> · <model> for ACP conversations (e.g. "Claude Code · Default (recommended)").
# Unit tests
npm test -- __tests__/scripts/dev-safe.test.ts __tests__/api/agent-server-adapter.test.ts __tests__/components/features/conversation-panel/conversation-card.test.tsx
# 127 passed
npm run lint
# clean

Video/Screenshots

To attach after a reviewer eyeballs the dev stack — the visible change is the chip rendering on the conversation list.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • Draft because the SDK pin is to an unmerged PR commit. Once software-agent-sdk#3347 merges and a new SDK release ships, the follow-up is: bump versions.agentServer in config/defaults.json to the new release and remove the agentServerGitRef field. The chip render path needs no further change once the SDK pin moves.
  • The existing conversation-card-acp-badge testid is gone; replaced by conversation-card-agent-chip. If any downstream automation greps for the old testid, this is a breaking rename — flagging in case.
  • Backward compat: agent-servers that predate the SDK PR don't lift current_model_* onto ConversationInfo. The adapter chain still resolves to null, and the chip falls back to the provider brand text — same behavior as today, just rendered with the new icon-first layout.
  • Per-provider brand marks (AgentBrandIcon) are nominative-use copies of the same path data the onboarding tile already ships under choose-agent-step.tsx. No new asset/licensing decisions.

🤖 Generated with Claude Code


🐳 Docker images for this PR

GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas

Component Value
Image ghcr.io/openhands/agent-canvas
Architectures amd64, arm64
Agent Server ghcr.io/openhands/agent-server:1.23.0-python
Automation openhands-automation==1.0.0a3
Commit 62a115a9e355fa02aac3602c7c00e0c00951fcbd

Pull (multi-arch manifest)

# Multi-arch manifest — Docker automatically pulls the correct architecture
docker pull ghcr.io/openhands/agent-canvas:sha-62a115a

Run

docker run -it --rm \
  -p 8000:8000 \
  ghcr.io/openhands/agent-canvas:sha-62a115a

All tags pushed for this build

ghcr.io/openhands/agent-canvas:sha-62a115a-amd64
ghcr.io/openhands/agent-canvas:feat-acp-model-chip-amd64
ghcr.io/openhands/agent-canvas:pr-708-amd64
ghcr.io/openhands/agent-canvas:sha-62a115a-arm64
ghcr.io/openhands/agent-canvas:feat-acp-model-chip-arm64
ghcr.io/openhands/agent-canvas:pr-708-arm64
ghcr.io/openhands/agent-canvas:sha-62a115a
ghcr.io/openhands/agent-canvas:feat-acp-model-chip
ghcr.io/openhands/agent-canvas:pr-708

About Multi-Architecture Support

  • Each tag (e.g., sha-62a115a) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., sha-62a115a-amd64) are also available if needed

For each conversation card, show a single inline chip:
  [brand mark]  <model text>

Mirrors OpenHands' main web UI conversation chip (PR #14510) so the two
products converge on the same identity affordance.

How the text is sourced:
- OpenHands native conversations: ``agent.llm.model`` (unchanged), gated
  behind the existing ``showLlmProfiles`` preference, rendered next to the
  OpenHands logo.
- ACP conversations: ``ConversationInfo.current_model_name`` lifted by the
  agent-server from software-agent-sdk PR #3347 (``ACPAgent.current_model_id``
  resolved through ``available_models`` to a human-readable name —
  ``"Default (recommended)"``, ``"GPT-5.5 (xhigh)"``, ``"Opus 4.1"``, …).
  Falls through to ``current_model_id`` then to the provider brand display
  name when older SDK builds don't lift either field. Hover tooltip always
  includes the harness identity for unambiguous attribution.

Pinning to the unreleased SDK PR:
- ``config/defaults.json`` gains an ``agentServerGitRef`` field set to the
  PR's latest commit (235fed007). ``scripts/dev-safe.mjs`` reads it and
  threads the SHA into the existing uvx git-ref install path so the dev
  stack picks up the new ``ConversationInfo.current_model_*`` fields without
  waiting for a PyPI release. Env vars
  ``OH_AGENT_SERVER_LOCAL_PATH`` / ``OH_AGENT_SERVER_GIT_REF`` /
  ``OH_AGENT_SERVER_VERSION`` still override; the in-repo default just
  promotes git-ref above the PyPI fallback. Clear the field once the SDK
  PR merges and a release ships.

Shape:
- New ``src/components/shared/agent-brand-icon.tsx`` renders the brand
  glyph per kind (``openhands`` | ``claude-code`` | ``codex`` | ``gemini``
  | ``cli-generic``) reusing the path data already inlined for the
  onboarding tile (extracted into ``src/constants/acp-brand-marks.ts``).
- ``resolveAcpProviderIcon(key)`` added alongside the existing display-name
  resolver.
- ``ConversationCardFooter`` now renders one chip via ``AgentBrandIcon`` +
  text + tooltip — replaces the standalone ACP pill and the separate
  LLM-model row.
- ``DirectConversationInfo`` typed for ``current_model_id`` /
  ``current_model_name``; the adapter chains
  ``current_model_name → current_model_id → null`` for ACP conversations
  and leaves OpenHands conversations on ``agent.llm.model``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agent-canvas Ready Ready Preview, Comment May 21, 2026 3:54pm

Request Review

The previous pin (235fed007) populated ``current_model_*`` only while
the ACP subprocess was live; idle / cold-listed conversations came back
with both fields null and the chip fell back to the provider brand
("Claude Code") instead of the model name. The new SDK commit persists
the resolved id + name into ``agent_state`` and reads them back on
cold conversation reads in ``_compose_conversation_info``, so the chip
shows the model on every ACP conversation in the sidebar.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@simonrosenberg simonrosenberg marked this pull request as ready for review May 21, 2026 15:03
github-actions Bot added a commit that referenced this pull request May 21, 2026
Copy link
Copy Markdown
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟢 Good taste - Clean implementation that solves the problem elegantly.

Summary

This PR successfully adds conversation chip functionality showing both agent harness brand marks and model names. The implementation is straightforward, well-tested, and maintains backward compatibility.

Key strengths:

  • Clean data structure with proper fallback chain: current_model_name → current_model_id → null
  • Comprehensive test coverage (127 tests passing)
  • Simple, maintainable component design
  • Good backward compatibility (optional fields, graceful degradation)
  • Dev workflow properly handles SDK git ref precedence

Notes:

  • ✅ Draft status is intentional (waiting for SDK PR #3347 to merge)
  • ✅ Breaking testid change (conversation-card-acp-badgeconversation-card-agent-chip) is documented
  • ✅ All tests pass, linting clean

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟢 LOW

This is a frontend UI enhancement with no security implications, no breaking API changes (only a testid rename that's documented), and comprehensive test coverage. The SDK pin to an unmerged PR is intentional for development and will be removed once the SDK PR ships.

VERDICT:
Worth merging once the SDK dependency (PR #3347) is merged and released. The code itself is ready.

KEY INSIGHT:
The adapter's field-chaining pattern (current_model_name ?? current_model_id ?? null) elegantly handles backward compatibility across three SDK generations without adding complexity.


Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/agent-canvas/actions/runs/26234359911

…name)

Previous pin (e6e9771) surfaced ``ModelInfo.name`` which for
claude-agent-acp's generic aliases is still an alias
("Default (recommended)") — not what's actually running. The new SDK
commit extracts the resolved model identity from ``ModelInfo.description``
for known aliases, so the chip now reads "Opus 4.7 with 1M context" /
"Sonnet 4.6" / "Haiku 4.5" instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #3347 alone tripped a pre-existing deadlock in
\`EventService._setup_stats_streaming\` (introduced in #3308) where the
stats_callback re-acquires the conversation state lock the caller
already holds, hanging every ACP turn before the assistant's
FinishAction is emitted. The new pin merges PR #3349 — which drops
the redundant \`with state:\` — into our PR branch so both the chip
data and the response flow work together. Drop both pins once #3347
and #3349 merge to main and a new SDK release ships.
@github-actions
Copy link
Copy Markdown
Contributor

📸 Snapshot Test Report

Warning

Snapshot comparison step crashed (timeout, OOM, or runner error) — diff results below may be incomplete or absent.
Check the CI logs for the full error output (look for the "Run snapshot comparison" step).

❌ 11 snapshots differ from the main branch baselines. Add the update-snapshots label to acknowledge intentional changes.

Category Count
🔴 Changed 11
🆕 New 0
✅ Unchanged 62
Total 73

How to resolve:

  • Unintentional diffs — the baselines on main may have moved since this branch was created. Merge the latest main into this branch and re-run CI.
  • Intentional changes — add the update-snapshots label. CI will pass and the new screenshots become the baseline when this PR merges.
🔴 Changed snapshots (11)

mcp-page — 5 snapshots

mcp-custom-server-1-editor-open

Expected (main) Actual (PR) Diff
expected actual diff

mcp-custom-server-editor

Expected (main) Actual (PR) Diff
expected actual diff

mcp-empty-installed

Expected (main) Actual (PR) Diff
expected actual diff

mcp-search-filtered

Expected (main) Actual (PR) Diff
expected actual diff

mcp-slack-install-1-marketplace

Expected (main) Actual (PR) Diff
expected actual diff

onboarding

onboarding-step-2-setup-llm

Expected (main) Actual (PR) Diff
expected actual diff

skills-page — 5 snapshots

skills-empty

Expected (main) Actual (PR) Diff
expected actual diff

skills-loaded

Expected (main) Actual (PR) Diff
expected actual diff

skills-no-match

Expected (main) Actual (PR) Diff
expected actual diff

skills-search-filtered

Expected (main) Actual (PR) Diff
expected actual diff

skills-type-filter

Expected (main) Actual (PR) Diff
expected actual diff
✅ Unchanged snapshots (62)

archived-conversation

  • conversation-panel-with-archived-badges
  • conversation-view-archived
  • conversation-view-sandbox-error

automations

  • automations-delete-modal
  • automations-list-active-inactive
  • automations-no-automations
  • automations-search-no-results

backends-extended

  • backend-add-blank-disabled
  • backend-add-cloud-advanced-open
  • backend-add-cloud-no-key-disabled
  • backend-add-cloud-with-key-enabled
  • backend-add-form-partially-filled
  • backend-add-invalid-url-disabled
  • backend-add-local-ready
  • backend-add-name-only-disabled
  • backend-add-two-column-layout
  • backend-add-whitespace-host-disabled
  • backend-after-switch
  • backend-cancel-nothing-saved
  • backend-dropdown-two-backends
  • backend-edit-prefilled
  • backend-manage-after-removal
  • backend-manage-two-listed
  • backend-remove-cancelled
  • backend-remove-confirmation
  • backend-switch-overlay

backends

  • backend-add-modal
  • backend-manage-modal
  • backend-selector-open

changes-tab

  • changes-deleted-file
  • changes-diff-viewer
  • changes-empty

collapsible-thinking

  • reasoning-content-collapsed
  • reasoning-content-expanded
  • think-action-collapsed
  • think-action-expanded

mcp-page

  • mcp-custom-server-2-url-filled
  • mcp-custom-server-3-all-filled
  • mcp-custom-server-4-installed
  • mcp-slack-install-2-modal
  • mcp-slack-install-3-filled
  • mcp-slack-install-4-installed

onboarding

  • onboarding-step-0-choose-agent
  • onboarding-step-1-check-backend
  • onboarding-step-3-say-hello

projects-workspace-browser

  • projects-workspace-browser

settings-page

  • add-backend-modal
  • analytics-consent-modal
  • home-screen
  • settings-app-page
  • settings-page

settings-secrets

  • secrets-add-form-filled
  • secrets-add-form
  • secrets-after-save
  • secrets-delete-confirm
  • secrets-list

settings-verification

  • condenser-settings
  • verification-settings-off
  • verification-settings-on

sidebar

  • sidebar-collapsed
  • sidebar-conversation-panel
  • sidebar-filter-menu

Generated by the Snapshot Tests workflow. This comment was created by an AI agent (OpenHands) on behalf of the repo maintainers.

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.

2 participants