fix(desktop): restore the authored description on persona cards - #36
Conversation
A sync merge left the persona card call site passing `card.personaLabel` as the card-face subtitle. That label is non-null only for a *split* card (where it disambiguates rivals); for a canonical card it is null, so the second line collapsed to empty and the persona's authored description was never shown. Upstream renders `effectiveAgentDescription(persona)` there. Result: editing a built-in agent's description saved the value but never displayed it — upstream's own regression test for block#1928 ("built-in persona edits persist") fails on our fork. Restore the fallback: a split card keeps its rival label, a canonical card falls back to the authored description, and the Hermes-profile suffix is unchanged. Evidence (verified against origin/product/main as the baseline): - agents.spec.ts:297 "built-in persona edits persist": FAILS on the base, PASSES with this change - agents.spec.ts (integration, full file): 37/37 pass - desktop unit suite: 6583/6583 pass - pnpm run check exit 0, pnpm run typecheck exit 0
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
| ref | run 1 | run 2 | run 3 |
|---|---|---|---|
origin/product/main (base) |
1 failed | 1 failed | 1 failed |
| this branch (#36) | 1 failed | 1 failed | 1 failed |
Identical. The failure exists on the base branch without this diff, so it is not a regression from #36.
Correcting my earlier analysis
I previously called this "CI shard-order pollution" and said it passed in isolation. That was wrong, and the better test disproves it:
- I found a plausible polluter (
message-feedback-snapshots.spec.tsrun before:3105reproducedReceived: 58), but the same pair then passed 2/2 — so that was coincidence, not cause. :3105also fails completely alone after a fresh build, with no other spec in the run.
There is no polluting test. The menuGap assertion (expect(menuGap).toBeLessThanOrEqual(headerPaddingInlineEnd + menuBox.width), 58 > 52) is environment-sensitive to layout/font metrics, and it fails that way on base too. The earlier "passes isolated" observations were environment-dependent, not code-dependent.
No layout code should be changed for this, and it should not gate this PR.
Problem
Editing a built-in agent's description saves the value but never displays it. Upstream's regression test for block#1928 —
agents.spec.ts:297 "built-in persona edits persist"— fails on our fork:The name (
My Fizz) persists; the description is simply absent from the card face.Root cause
A sync merge left the call site in
UnifiedAgentsSection.tsxpassingcard.personaLabelas the subtitle. That label is non-null only for a split card (disambiguating rivals); for a canonical card it isnull(unifiedAgentGroups.ts:72), soruntimeSubtitlecollapsed to empty.Upstream computes that second line as
effectiveAgentDescription(persona) ?? modelLabel. The helper still exists in our tree and is used elsewhere — only this call site lost it.This is the fourth upstream guarantee broken by fork-local drift and surfaced by a byte-identical upstream spec.
Fix
Fall back to the authored description when no split label is supplied. Split cards keep their rival label; the
Hermes · <profile>suffix is unchanged.Evidence
Baseline established by running the same test on
origin/product/mainwithout this diff:agents.spec.ts:297— FAILS on base, passes with this changeagents.spec.tsfull file (integration) — 37/37 passpnpm run checkexit 0 ·pnpm run typecheckexit 0Note
Confirms PR #34 did not cause this integration failure — it reproduces identically on the base.