mirror of
https://github.com/block/buzz.git
synced 2026-11-12 20:31:12 +00:00
fix(desktop): apply the focus code recipe to fenced human prompts too
Quality review caught a real bug in the previous commit. The `CodeBlockVariantContext` provider was mounted inside `MessageActivity`, which only handles assistant items — `UserMessageBubble` returns before it. A fence inside a human prompt therefore kept the legacy 16px muted frame, nested inside the new 12px prompt bubble. The recipe is a property of the *surface*, not of a role: a fenced block in a prompt should read the same as one in a reply. The provider moves up to `AgentSessionTranscriptList`, alongside the variant and turn-meta providers, so every descendant markdown render inherits it. It still renders no DOM, so `default`/`compactPreview` markup stays byte-identical and the baseline fixture is unchanged. Added a fenced-prompt test, which the existing fenced test could not catch because it only rendered an assistant item. Proven non-vacuous two ways: pinning the boundary provider to `default` fails both fenced tests, and re-adding a role-scoped `default` provider around only the user bubble's markdown fails the new prompt test alone. Also renamed `channel-message code blocks are untouched by the focus recipe` to `the default transcript variant keeps the legacy code chrome`. It renders the `default` transcript variant, not a channel message row, so the old name claimed a contract it did not prove. AGENTS.md gains the `assert.equal(el, null)` note: that form serializes the matched element's whole subtree to build a failure diff and OOMs the runner instead of printing the message, which made a genuine failure unreadable. Use `assert.ok(x === null)`. Verified at this tree: full desktop suite 5428 passing / 0 failing, `tsc --noEmit` clean, `biome check` at main's baseline, px-text / pubkey-truncation / file-size clean. Screenshots captured light and dark by temporarily pinning `transcriptVariant="conversation"` in `AgentSessionThreadPanel`; that pin is dev-00's slice and is not in this commit. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
18
AGENTS.md
18
AGENTS.md
@@ -256,6 +256,24 @@ scripts build the required E2E bridge before running Playwright.
|
||||
|
||||
See [TESTING.md](TESTING.md) for the full multi-agent E2E guide.
|
||||
|
||||
### Never assert a DOM element against `null` with `assert.equal`
|
||||
|
||||
In the desktop jsdom tests (`desktop/src/**/*.test.mjs`), write
|
||||
|
||||
```js
|
||||
assert.ok(container.querySelector(sel) === null, "no header row");
|
||||
```
|
||||
|
||||
**not** `assert.equal(container.querySelector(sel), null, ...)`. Both pass
|
||||
identically, but when the `assert.equal` form *fails*, node serializes the
|
||||
matched element and its entire subtree to build a diff. On a real transcript
|
||||
that exhausts memory and the runner dies with SIGKILL after ~100s instead of
|
||||
printing the assertion message — so a genuine regression is unreadable and
|
||||
looks like a hang or an OOM in unrelated code. The `assert.ok(x === null)` form
|
||||
fails in milliseconds with the message you wrote.
|
||||
|
||||
Comparing `getAttribute(...)` to `null` is fine — attributes are strings.
|
||||
|
||||
### PR Screenshots
|
||||
|
||||
> **Do NOT use `buzz upload`, the relay media endpoint, or any third-party
|
||||
|
||||
@@ -435,6 +435,26 @@ function fencedCodeItems() {
|
||||
];
|
||||
}
|
||||
|
||||
/** One *human prompt* whose body is a fenced code block. */
|
||||
function fencedCodePromptItems() {
|
||||
return [
|
||||
{
|
||||
channelId: "chan-1",
|
||||
sessionId: "sess-1",
|
||||
turnId: "turn-1",
|
||||
id: "msg:user",
|
||||
type: "message",
|
||||
renderClass: "message",
|
||||
role: "user",
|
||||
title: TRIGGER_TITLE,
|
||||
text: "fix this\n\n```ts\nconst a = 1;\nconst b = 2;\n```\n",
|
||||
timestamp: "2026-06-14T19:00:00.000Z",
|
||||
messageId: "event-1",
|
||||
authorPubkey: AUTHOR,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Same mount, but the caller can swap the list props afterwards. Needed for the
|
||||
* contracts that are only visible across a rerender: a streaming thought
|
||||
@@ -619,11 +639,42 @@ test("conversation frames fenced code with berd's header row", async () => {
|
||||
assert.equal(code.querySelectorAll("[data-line]").length, 2);
|
||||
});
|
||||
|
||||
test("channel-message code blocks are untouched by the focus recipe", async () => {
|
||||
test("conversation applies the code recipe to a fenced human prompt too", async () => {
|
||||
// Regression guard for a real bug quality caught. The provider was first
|
||||
// mounted inside `MessageActivity`, which only handles assistant items — the
|
||||
// user bubble returns before it, so a fence inside a prompt kept the legacy
|
||||
// 16px muted frame nested inside the new 12px bubble. The recipe is a
|
||||
// property of the *surface*, not of a role, so the provider now sits at the
|
||||
// transcript boundary and both roles inherit it.
|
||||
const { container } = await renderTranscriptWithCodeChrome("conversation", {
|
||||
items: fencedCodePromptItems(),
|
||||
});
|
||||
const bubble = container.querySelector(
|
||||
'[data-testid="transcript-user-message"]',
|
||||
);
|
||||
assert.ok(bubble, "the prompt should render");
|
||||
assert.ok(
|
||||
bubble.querySelector('[data-testid="markdown-code-block-header"]'),
|
||||
"a fence inside the prompt gets berd's header row",
|
||||
);
|
||||
const frame = bubble.querySelector("pre");
|
||||
assert.match(frame.className, /rounded-lg/);
|
||||
assert.doesNotMatch(
|
||||
frame.className,
|
||||
/rounded-2xl/,
|
||||
"the legacy 16px frame must not nest inside the 12px bubble",
|
||||
);
|
||||
assert.doesNotMatch(frame.className, /pr-12/);
|
||||
});
|
||||
|
||||
test("the default transcript variant keeps the legacy code chrome", async () => {
|
||||
// The markdown renderer is shared with channel messages, so `focusProse` is
|
||||
// opt-in per surface. Rendering the same fenced block through `default` must
|
||||
// still produce the original chrome: no header row, 16px radius, muted fill,
|
||||
// and the absolutely-positioned copy button.
|
||||
// opt-in per surface. Rendering the same fenced block through the `default`
|
||||
// transcript variant must still produce the original chrome: no header row,
|
||||
// 16px radius, muted fill, and the absolutely-positioned copy button.
|
||||
//
|
||||
// This proves the *variant gate*, not the channel-message row itself — those
|
||||
// rows are covered by the markdown tests in `shared/ui/markdown`.
|
||||
const { container } = await renderTranscriptWithCodeChrome("default", {
|
||||
items: fencedCodeItems(),
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@ import { useStableArrayShallow } from "@/shared/hooks/useStableReference";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { AnimatedCount } from "@/shared/ui/AnimatedCount";
|
||||
import { FuzzyLogo } from "@/shared/ui/buzz-logo/FuzzyLogo";
|
||||
import { CodeBlockVariantContext } from "@/shared/ui/markdown/CodeBlock";
|
||||
import type { TranscriptItem } from "./agentSessionTypes";
|
||||
import { TurnLivenessIndicator } from "./TurnLivenessIndicator";
|
||||
import {
|
||||
@@ -229,41 +230,53 @@ export function AgentSessionTranscriptList({
|
||||
ref={autoTail ? contentRef : undefined}
|
||||
role="log"
|
||||
>
|
||||
<AgentSessionTranscriptVariantProvider value={variant}>
|
||||
<AgentSessionTranscriptTurnMetaProvider value={turnMeta}>
|
||||
{displayBlocks.map((block) => {
|
||||
const blockKey = getDisplayBlockKey(block);
|
||||
return (
|
||||
<motion.div
|
||||
animate={ROW_ENTER_TO}
|
||||
data-message-id={blockKey}
|
||||
initial={
|
||||
animationsDisabled || !hasCompletedInitialRenderRef.current
|
||||
? false
|
||||
: ROW_ENTER_FROM
|
||||
}
|
||||
key={blockKey}
|
||||
layout={layoutAnimationsEnabled ? "position" : false}
|
||||
transition={ROW_ENTER_SPRING}
|
||||
>
|
||||
{/* content-visibility stays on a non-animated child: motion
|
||||
{/* The berd code-block recipe is a property of the *surface*, not of a
|
||||
role: a fenced block in a human prompt must get the same chrome as
|
||||
one in an agent reply. Provided at the transcript boundary so every
|
||||
descendant markdown render inherits it. A context provider emits no
|
||||
DOM, so `default`/`compactPreview` markup is unaffected. */}
|
||||
<CodeBlockVariantContext.Provider
|
||||
value={isConversation ? "focusProse" : "default"}
|
||||
>
|
||||
<AgentSessionTranscriptVariantProvider value={variant}>
|
||||
<AgentSessionTranscriptTurnMetaProvider value={turnMeta}>
|
||||
{displayBlocks.map((block) => {
|
||||
const blockKey = getDisplayBlockKey(block);
|
||||
return (
|
||||
<motion.div
|
||||
animate={ROW_ENTER_TO}
|
||||
data-message-id={blockKey}
|
||||
initial={
|
||||
animationsDisabled ||
|
||||
!hasCompletedInitialRenderRef.current
|
||||
? false
|
||||
: ROW_ENTER_FROM
|
||||
}
|
||||
key={blockKey}
|
||||
layout={layoutAnimationsEnabled ? "position" : false}
|
||||
transition={ROW_ENTER_SPRING}
|
||||
>
|
||||
{/* content-visibility stays on a non-animated child: motion
|
||||
measures the outer wrapper for layout animations, which
|
||||
would otherwise force skipped offscreen rows to render. */}
|
||||
<div className="content-visibility-auto">
|
||||
<TranscriptDisplayBlockView
|
||||
agentAvatarUrl={agentAvatarUrl}
|
||||
agentName={agentName}
|
||||
agentPubkey={agentPubkey}
|
||||
block={block}
|
||||
profiles={profiles}
|
||||
/>
|
||||
</div>
|
||||
</motion.div>
|
||||
);
|
||||
})}
|
||||
{isTurnLive && !isCompactPreview ? <TurnLivenessIndicator /> : null}
|
||||
</AgentSessionTranscriptTurnMetaProvider>
|
||||
</AgentSessionTranscriptVariantProvider>
|
||||
<div className="content-visibility-auto">
|
||||
<TranscriptDisplayBlockView
|
||||
agentAvatarUrl={agentAvatarUrl}
|
||||
agentName={agentName}
|
||||
agentPubkey={agentPubkey}
|
||||
block={block}
|
||||
profiles={profiles}
|
||||
/>
|
||||
</div>
|
||||
</motion.div>
|
||||
);
|
||||
})}
|
||||
{isTurnLive && !isCompactPreview ? (
|
||||
<TurnLivenessIndicator />
|
||||
) : null}
|
||||
</AgentSessionTranscriptTurnMetaProvider>
|
||||
</AgentSessionTranscriptVariantProvider>
|
||||
</CodeBlockVariantContext.Provider>
|
||||
</div>
|
||||
</motion.div>
|
||||
);
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import { Markdown } from "@/shared/ui/markdown";
|
||||
import { CodeBlockVariantContext } from "@/shared/ui/markdown/CodeBlock";
|
||||
import { UserAvatar } from "@/shared/ui/UserAvatar";
|
||||
import { useAgentSessionTranscriptVariant } from "../agentSessionTranscriptContext";
|
||||
import { formatTranscriptTimestampTitle } from "../agentSessionUtils";
|
||||
@@ -99,25 +98,19 @@ function MessageItem({
|
||||
}
|
||||
title={formatTranscriptTimestampTitle(item.timestamp)}
|
||||
>
|
||||
{/* A context provider renders no DOM, so wrapping unconditionally
|
||||
keeps `default`/`compactPreview` markup byte-identical. */}
|
||||
<CodeBlockVariantContext.Provider
|
||||
value={isConversation ? "focusProse" : "default"}
|
||||
>
|
||||
<Markdown
|
||||
className={
|
||||
isCompactPreview
|
||||
? "text-xs leading-4"
|
||||
: isConversation
|
||||
? // Focus mode reads as prose: no box, comfortable line
|
||||
// height, and full-fidelity markdown/code from the shared
|
||||
// renderer.
|
||||
"leading-relaxed"
|
||||
: "leading-5"
|
||||
}
|
||||
content={text || " "}
|
||||
/>
|
||||
</CodeBlockVariantContext.Provider>
|
||||
<Markdown
|
||||
className={
|
||||
isCompactPreview
|
||||
? "text-xs leading-4"
|
||||
: isConversation
|
||||
? // Focus mode reads as prose: no box, comfortable line
|
||||
// height, and full-fidelity markdown/code from the shared
|
||||
// renderer.
|
||||
"leading-relaxed"
|
||||
: "leading-5"
|
||||
}
|
||||
content={text || " "}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -28,7 +28,10 @@ import { getReactNodeText } from "./utils";
|
||||
* border, and no shadow.
|
||||
*
|
||||
* It is opt-in because this renderer is shared with channel messages —
|
||||
* restyling the default would change every code block in the app.
|
||||
* restyling the default would change every code block in the app. The variant
|
||||
* is a property of the *surface*, not of a role: `AgentSessionTranscriptList`
|
||||
* provides it once at the transcript boundary, so a fence in a human prompt
|
||||
* gets the same chrome as one in an agent reply.
|
||||
*
|
||||
* Delivered through context rather than a `MarkdownCodeBlock` prop for the same
|
||||
* reason as `VideoReviewMarkdownContext`: the component map handed to
|
||||
|
||||
Reference in New Issue
Block a user