Skip to content

feat(agents): applications landing polish + chat dedup fix#2742

Open
dmarticus wants to merge 2 commits into
posthog-code/m3a-finalfrom
dylan/agent-builder
Open

feat(agents): applications landing polish + chat dedup fix#2742
dmarticus wants to merge 2 commits into
posthog-code/m3a-finalfrom
dylan/agent-builder

Conversation

@dmarticus

Copy link
Copy Markdown
Contributor

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-final so the diff stays scoped. Merge order: #2700 → this.

Summary

Landing polish (commit 7a2094ed9)

  • Single entry point to the Agent Builder. Dropped the contextual New agent / Edit with AI header 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. Deleted EditWithAIButton + agentBuilderActions.
  • Tinted the dock with a subtle amber accent (left border + faint header wash) so it reads as the meta-tool rather than another task chat. Mirrors the existing SparkleIcon accent.
  • Reordered the Applications landing. Agents list moves to the top, sectioned LIVE vs DRAFTS (drafts dimmer, count in the section label, hidden when empty). Activity rollup hides when 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.
  • Tightened the Applications shell copy to mirror Scouts: "Deployed agents triggered by chat, Slack, webhooks, or cron — preview, approve, and edit with the Agent Builder." (single line at the current width).

Chat dedup fix (commit c5cda4811)

The optimistic-seed → SSE-echo dedup in createAgentChatMapper used strict equality on pendingOptimistic[0], which broke in three real failure modes — each one surfaced the same way: the user's just-sent bubble rendered twice.

Failure mode Symptom Fix
Whitespace asymmetry Runner echoes back with trailing \n or padding around the context envelope; strict equality misses by one character. Compare text.trim() to pending.trim().
Out-of-order echo Rapid back-to-back sends land out of order; [0] check only inspects the head. findIndex over the whole queue.
Runner re-emit Runner emits the same user_message twice; second arrival has nothing in pendingOptimistic to swallow it. Track every rendered user text in a seenUserTexts set; drop later duplicates.

Four new tests in sessionEventToAcp.test.ts covering each failure mode. 15/15 pass; pnpm --filter @posthog/ui typecheck clean.

Test plan

  • Apps list looks right: LIVE/DRAFTS sections, drafts dimmer, hidden when empty.
  • Open the dock; it has the amber accent and no longer competes with a page-header CTA.
  • Pending-approvals link still routes to /code/agents/applications/approvals and tints amber when non-zero.
  • Live now panel renders its own empty state when the fleet is quiet.
  • Send a message in the Agent Builder dock as a fresh session — user bubble appears once (not twice).
  • Send a message in a per-agent Chat preview tab — same.
  • Send two messages back-to-back fast — both render exactly once each.

🤖 Generated with Claude Code

dmarticus and others added 2 commits June 17, 2026 14:12
- 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>
@github-actions

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit c5cda48.

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix 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

Comment on lines +52 to +64
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([]);
});

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.

P2 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.

Suggested change
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!

Comment on lines 261 to 265
/**
* 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.
*/

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.

P2 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.

Suggested change
/**
* 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'll PR against it with what i think is preferable

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