feat(agents): applications landing polish + chat dedup fix#2742
feat(agents): applications landing polish + chat dedup fix#2742dmarticus wants to merge 2 commits into
Conversation
- Drop the contextual "New agent / Edit with AI" header CTA on every agents view; the always-on sparkles dock toggle is the single entry point. The dock toggle adds aria + tooltip and a "Following" badge while the builder is mid-turn. Delete EditWithAIButton + agentBuilderActions (only callers were inside this header). - Tint the Agent Builder dock with a subtle amber accent (left border + faint header wash) so it reads as the meta-tool rather than another task chat. Header label and sparkle icon were already amber-accent. - Reorder the Applications landing: agents list moves to the top, sectioned LIVE vs DRAFTS (drafts dimmer with count in the section label, hidden when empty). The activity rollup hides when analytics.empty, and the operational strip drops the live-count chip (the Live now panel below already owns that signal) and renders only the pending-approvals link, amber-tinted when non-zero. - Always render the Live now panel; it owns its own "Nothing running" empty state. - Tighten the per-tab description copy on the Agents shell: shorter Applications line that mirrors the Scouts shape — "[noun] that [what they do] — [what you do here]" — and fits on one line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The optimistic-seed → SSE-echo dedup in createAgentChatMapper compared strict-equal on `pendingOptimistic[0]`, which broke in three real failure modes — all of which surfaced the same way: the user's just- sent bubble rendered twice. - Whitespace asymmetry. The composer trims before seeding, but the runner sometimes echoes back with a trailing `\n` or padding around the agent-builder context envelope. Strict equality misses by one character. - Out-of-order arrival. Echoes from rapid back-to-back sends can land out of order; the `[0]` check only inspects the head of the queue. - Runner re-emit. Nothing guarded against the runner emitting the same `user_message` event twice — the second arrival had nothing in `pendingOptimistic` to swallow it and fell through to a fresh render. The mapper now (1) compares trimmed forms, (2) findIndex's the pending queue rather than only `[0]`, and (3) tracks every rendered user text in a `seenUserTexts` set so any later duplicate `user_message` is suppressed even after `pendingOptimistic` is drained. Tests: four new cases covering the trailing-whitespace, out-of-order, and runner-re-emit failure modes plus a baseline echo-swallow. 15/15 in sessionEventToAcp.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/ui/src/features/agent-applications/chat/sessionEventToAcp.test.ts:52-64
Per the repo's preference for parameterised tests, the first two new dedup tests probe the same behaviour (seed + echo → swallowed) with different string variants. Collapsing them into a single `it.each` table is more concise and makes adding more whitespace variants (e.g., leading spaces, both ends) trivial.
```suggestion
it.each([
["exact match", "hello", "hello"],
["trailing newline", "hello", "hello\n"],
["leading spaces", "hello", " hello"],
])(
"swallows the echo of an optimistically-seeded message (%s)",
(_, seeded, echoed) => {
const mapper = createAgentChatMapper();
mapper.seedUserMessage(seeded);
expect(mapper.apply(ev("user_message", { text: echoed }))).toEqual([]);
},
);
```
### Issue 2 of 2
packages/ui/src/features/agent-applications/components/AgentApplicationsListView.tsx:261-265
The JSDoc says "only rendered when something is actually happening," but `OperationalStrip` is called unconditionally in the parent and renders even when `pendingCount === 0` (showing "0 pending approvals" as a navigation link). The comment should be updated to reflect actual behaviour.
```suggestion
/**
* Operational counts strip — always rendered as a persistent navigation link
* to the fleet approvals queue. Tinted amber when there are pending approvals,
* neutral otherwise.
*/
```
Reviews (1): Last reviewed commit: "fix(agents): dedup user_message echoes r..." | Re-trigger Greptile |
| it("swallows the echo of an optimistically-seeded message", () => { | ||
| const mapper = createAgentChatMapper(); | ||
| mapper.seedUserMessage("hello"); | ||
| expect(mapper.apply(ev("user_message", { text: "hello" }))).toEqual([]); | ||
| }); | ||
|
|
||
| it("swallows the echo even when the runner adds trailing whitespace", () => { | ||
| const mapper = createAgentChatMapper(); | ||
| mapper.seedUserMessage("hello"); | ||
| // Runners commonly normalize by adding a trailing newline — that mustn't | ||
| // break dedup or the user sees their bubble twice. | ||
| expect(mapper.apply(ev("user_message", { text: "hello\n" }))).toEqual([]); | ||
| }); |
There was a problem hiding this comment.
Per the repo's preference for parameterised tests, the first two new dedup tests probe the same behaviour (seed + echo → swallowed) with different string variants. Collapsing them into a single
it.each table is more concise and makes adding more whitespace variants (e.g., leading spaces, both ends) trivial.
| it("swallows the echo of an optimistically-seeded message", () => { | |
| const mapper = createAgentChatMapper(); | |
| mapper.seedUserMessage("hello"); | |
| expect(mapper.apply(ev("user_message", { text: "hello" }))).toEqual([]); | |
| }); | |
| it("swallows the echo even when the runner adds trailing whitespace", () => { | |
| const mapper = createAgentChatMapper(); | |
| mapper.seedUserMessage("hello"); | |
| // Runners commonly normalize by adding a trailing newline — that mustn't | |
| // break dedup or the user sees their bubble twice. | |
| expect(mapper.apply(ev("user_message", { text: "hello\n" }))).toEqual([]); | |
| }); | |
| it.each([ | |
| ["exact match", "hello", "hello"], | |
| ["trailing newline", "hello", "hello\n"], | |
| ["leading spaces", "hello", " hello"], | |
| ])( | |
| "swallows the echo of an optimistically-seeded message (%s)", | |
| (_, seeded, echoed) => { | |
| const mapper = createAgentChatMapper(); | |
| mapper.seedUserMessage(seeded); | |
| expect(mapper.apply(ev("user_message", { text: echoed }))).toEqual([]); | |
| }, | |
| ); |
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/agent-applications/chat/sessionEventToAcp.test.ts
Line: 52-64
Comment:
Per the repo's preference for parameterised tests, the first two new dedup tests probe the same behaviour (seed + echo → swallowed) with different string variants. Collapsing them into a single `it.each` table is more concise and makes adding more whitespace variants (e.g., leading spaces, both ends) trivial.
```suggestion
it.each([
["exact match", "hello", "hello"],
["trailing newline", "hello", "hello\n"],
["leading spaces", "hello", " hello"],
])(
"swallows the echo of an optimistically-seeded message (%s)",
(_, seeded, echoed) => {
const mapper = createAgentChatMapper();
mapper.seedUserMessage(seeded);
expect(mapper.apply(ev("user_message", { text: echoed }))).toEqual([]);
},
);
```
**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /** | ||
| * Operational counts strip — restores the "live now / pending approvals" | ||
| * signals the M7 analytics KPIs displaced. Live count anchors the live-now | ||
| * panel below; pending links to the fleet approvals queue. | ||
| * Operational counts strip — only rendered when something is actually | ||
| * happening. Pending deep-links to the fleet approvals queue; live anchors the | ||
| * live-now panel below. | ||
| */ |
There was a problem hiding this comment.
The JSDoc says "only rendered when something is actually happening," but
OperationalStrip is called unconditionally in the parent and renders even when pendingCount === 0 (showing "0 pending approvals" as a navigation link). The comment should be updated to reflect actual behaviour.
| /** | |
| * Operational counts strip — restores the "live now / pending approvals" | |
| * signals the M7 analytics KPIs displaced. Live count anchors the live-now | |
| * panel below; pending links to the fleet approvals queue. | |
| * Operational counts strip — only rendered when something is actually | |
| * happening. Pending deep-links to the fleet approvals queue; live anchors the | |
| * live-now panel below. | |
| */ | |
| /** | |
| * Operational counts strip — always rendered as a persistent navigation link | |
| * to the fleet approvals queue. Tinted amber when there are pending approvals, | |
| * neutral otherwise. | |
| */ |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/agent-applications/components/AgentApplicationsListView.tsx
Line: 261-265
Comment:
The JSDoc says "only rendered when something is actually happening," but `OperationalStrip` is called unconditionally in the parent and renders even when `pendingCount === 0` (showing "0 pending approvals" as a navigation link). The comment should be updated to reflect actual behaviour.
```suggestion
/**
* Operational counts strip — always rendered as a persistent navigation link
* to the fleet approvals queue. Tinted amber when there are pending approvals,
* neutral otherwise.
*/
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Personally i think this is a regresssion :( Having the clear prompt to push to the agent builder makes a lot of sense. Otherwise it isn't at all clear that this is even an option.
The sparkle button isn't obvious enough imo and i think this change isn't worth it
There was a problem hiding this comment.
I'll PR against it with what i think is preferable
Polish on top of #2700 — focused on the Applications landing IA, the Agent Builder dock's distinct identity, and a real bug in the chat preview's user-message dedup.
Stacked on top of
posthog-code/m3a-finalso the diff stays scoped. Merge order: #2700 → this.Summary
Landing polish (commit
7a2094ed9)New agent/Edit with AIheader CTA across every agents view; the always-on sparkles dock toggle is now the only entry point. Removes the "two near-identical gold buttons" confusion in the page header. DeletedEditWithAIButton+agentBuilderActions.SparkleIconaccent.analytics.empty. Operational strip drops the live-count chip — the always-visible Live now panel below owns that signal — and keeps only the pending-approvals link, amber-tinted when non-zero.Chat dedup fix (commit
c5cda4811)The optimistic-seed → SSE-echo dedup in
createAgentChatMapperused strict equality onpendingOptimistic[0], which broke in three real failure modes — each one surfaced the same way: the user's just-sent bubble rendered twice.\nor padding around the context envelope; strict equality misses by one character.text.trim()topending.trim().[0]check only inspects the head.findIndexover the whole queue.user_messagetwice; second arrival has nothing inpendingOptimisticto swallow it.seenUserTextsset; drop later duplicates.Four new tests in
sessionEventToAcp.test.tscovering each failure mode. 15/15 pass;pnpm --filter @posthog/ui typecheckclean.Test plan
/code/agents/applications/approvalsand tints amber when non-zero.🤖 Generated with Claude Code