Skip to content

[lenny] fix(desktop): show authored persona description on agent cards - #33

Closed
mfethe1 wants to merge 1 commit into
product/mainfrom
fix/lenny/persona-description-card
Closed

[lenny] fix(desktop): show authored persona description on agent cards#33
mfethe1 wants to merge 1 commit into
product/mainfrom
fix/lenny/persona-description-card

Conversation

@mfethe1

@mfethe1 mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Problem

The agents library card never showed an owner-authored persona description. Editing a
persona's description saved correctly (the list_personas round-trip keeps it), but the
card's second line kept showing Default model (...), so the feature was invisible in the
one place it was designed for.

This is why built-in persona edits persist (desktop/tests/e2e/agents.spec.ts:297) has
been failing on every PR in this fork, including PRs that touch no application code at
all. It is trunk breakage, not a defect in any individual PR.

Root cause — a semantic merge conflict

Upstream block#7126 ("add public descriptions to agent personas") renamed
AgentIdentityCard's modelLabel prop to subtitle, documenting the contract as:

the effective description when one resolves (owner-authored — see lib/agentDescription.ts),
otherwise the model label. Callers compose the fallback.

Our fork's split-card work (3acf014ab3, "give a split persona card a name and a stable
identity") independently started passing personaLabel as that same subtitle, and
composes it into runtimeSubtitle alongside the Hermes profile.

Both changes are individually correct. Merged, the fork's caller never composes the
description, so the upstream contract is silently unmet — a textual clean merge with a
behavioral conflict.

Fix

Compose the documented fallback in AgentPersonaCard: authored description first, then the
persona label a split card carries. The Hermes · <profile> suffix is preserved either way,
so a machine-homed agent still shows which runtime it lives on.

Verification

Run locally on this branch:

  • built-in persona edits persist (integration) — 1 passed (was: failed 3/3 attempts,
    Expected substring: "Helps teams ship reliably." / Received: "MFStartMy FizzDefault model (claude-opus-4-5)")
  • unifiedAgentGroups.test.mjs20/20 pass
  • pnpm run check (typecheck + biome + repo guards, all workspaces) — EXIT=0
  • pnpm run test (desktop unit) — 6584 tests, 6581 pass. The 3 failures
    (useKnownAgentPubkeys, ChannelTaskList, relaySyncRequiredPolicy) are pre-existing:
    verified by stashing this diff and re-running them, where they fail identically.

⚠️ Note for reviewers: the Playwright harness serves the prebuilt dist/
(webServer: python3 -m http.server 4173 -d dist) and does not rebuild. Run
pnpm run build:e2e before the e2e suite or you will test stale JS — this initially made a
working fix look like a failing one.

Out of scope

narrow thread view collapses channel header actions into a menu
(messaging.spec.ts:3142) also fails at trunk and is not addressed here. It asserts
menuGap <= headerPaddingInlineEnd + menuBox.width (58 vs 52) against a header that renders
correctly in the failure screenshot — an over-tight geometric assertion in a separate
concern. Filed separately rather than mixed into this fix.

The agents library card rendered the persona display name (or nothing) as its second line, so an owner-authored description never appeared - the built-in persona edit round-trip saved the description but the card kept showing the model label.

Upstream block#7126 renamed AgentIdentityCard's modelLabel to subtitle with the contract 'the effective description when one resolves, otherwise the model label - callers compose the fallback'. The fork's split-card work landed its own personaLabel subtitle and never composed that fallback, so the two changes were individually correct but semantically conflicting after the merge.

Compose the documented fallback: effectiveAgentDescription(persona) then the persona label, keeping the Hermes profile suffix so machine-homed agents still show their runtime.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5035c5f8-b637-466c-87d9-b7ababa4e469

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

CI attribution: all remaining red is pre-existing on trunk

This PR changes one file (desktop/src/features/agents/ui/UnifiedAgentsSection.tsx,
one subtitle composition + one import). Every remaining red check was reproduced on a
pristine trunk worktree with no local edits.

Reproduction method

git worktree add /tmp/trunk-check origin/product/main   # adeec6eb5c, git status EMPTY
cd /tmp/trunk-check/desktop
pnpm install --frozen-lockfile        # INSTALL=0
pnpm run build:e2e                    # BUILD=0

Note: the Playwright harness serves a prebuilt dist/ (python3 -m http.server -d dist)
and never rebuilds. pnpm run build:e2e is mandatory before any local E2E run, or you are
testing stale JavaScript.

Failures proven pre-existing (no changes applied)

Spec Result on pristine trunk
exact-key-profile.spec.ts:84 archived exact key stays navigable 1 failed / 1 passed
spoiler.spec.ts:256 masked link does not leak URL until revealed 1 failed / 10 passed
messaging.spec.ts:3142 narrow thread view collapses header actions fails (gap 58 > allowance 52)
thread-reply-anchor-roleplay.spec.ts (3 cases) passes locally, fails only in CI — same toHaveCount Expected:0 Received:1 pattern → CI-environment flake

Unit suite: 3 of 6584 fail (useKnownAgentPubkeys, ChannelTaskList, relaySyncRequiredPolicy).
Proven pre-existing by stashing this diff out and re-running — failures persist unchanged.

What this PR actually fixes

Desktop E2E Integration / built-in persona edits persist (agents.spec.ts:297) went from
failing → passing. That check is red on other open PRs and green here.

Green on this branch

  • pnpm run checkEXIT=0 (typecheck, biome, file-size ratchet, px-text, pubkey-truncation,
    pubkey-normalization, agent-identity, admin-web vitest)
  • unifiedAgentGroups unit tests → 20/20 pass
  • built-in persona edits persist1 passed, EXIT=0 (after build:e2e)

None of the listed failures touch persona rendering, agent cards, or this file.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

CI attribution: the Desktop E2E Integration red is pre-existing, not from this diff

This PR is a 1-file, +8/-1 change to UnifiedAgentsSection.tsx. Its only remaining red is Desktop E2E Integration, failing one test:

profile.spec.ts:1875 › an older agent message stays exact while persona navigation selects the live instance
locator.click: Test timeout of 30000ms exceeded — waiting for getByTestId('persona-agent-row-profile-parity-agent')

That same test fails identically on three sibling PRs that never touch this file:

PR touches UnifiedAgentsSection.tsx shard 1 shard 2 failing test on shard 2
#33 (this) yes ✅ SUCCESS an older agent message stays exact…
#29 no an older agent message stays exact…
#31 no an older agent message stays exact…
#32 no an older agent message stays exact…

Sibling PRs fail both shards (#29 and #32 additionally fail built-in persona edits persist). This PR is the only one of the four whose shard 1 passes — so on this check it is strictly healthier than the comparison set, not worse.

Why the diff cannot be the cause:

  • The change is null-safe: effectiveAgentDescription(persona) ?? subtitle — the helper returns string | null and the previous value is used as the fallback. It cannot throw and cannot prevent the row from rendering.
  • The failing locator is persona-agent-row-${card.key} (line 196), which this diff does not touch — verified with git show 865a695f21 | grep persona-agent-row → no matches.
  • The imported module @/features/agents/lib/agentDescription exists and exports effectiveAgentDescription; a bad import would have failed the build, and Desktop Build (macOS) is ✅.

Conclusion: the failure is a pre-existing/flaky condition in the shard-2 integration environment affecting all open PRs on this base, not a regression introduced here. Requesting review on that basis; I am not merging on a red gate.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #36, which lands the same effectiveAgentDescription fallback in the same file.

Both PRs add the identical import and touch the same runtimeSubtitle line, so only one can land. Choosing #36 on precedence: it uses subtitle ?? effectiveAgentDescription(persona), whereas this PR uses effectiveAgentDescription(persona) ?? subtitle.

A split card passes its rival name as subtitle specifically to disambiguate two same-named personas. Description-first would overwrite that disambiguator whenever a description exists, so split cards could render two visually identical second lines. Subtitle-first keeps the disambiguation and still fixes the reported bug, since a canonical card passes no subtitle and therefore falls through to the authored description.

No work is lost — the behaviour you documented for canonical cards ships in #36.

@mfethe1 mfethe1 closed this Sep 11, 2026
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.

1 participant