Skip to content

feat(canvas): collapse canvas generation instructions into a clickable chip#2883

Merged
raquelmsmith merged 2 commits into
mainfrom
posthog-code/canvas-instructions-chip
Jun 23, 2026
Merged

feat(canvas): collapse canvas generation instructions into a clickable chip#2883
raquelmsmith merged 2 commits into
mainfrom
posthog-code/canvas-instructions-chip

Conversation

@raquelmsmith

Copy link
Copy Markdown
Member

Problem

When generating a freeform canvas, the agent's first user message dumped the entire canvas authoring contract + publishing/data rules inline as plain text — boilerplate the user never typed, drowning out their actual request.

Why: In a project-bluebird channel, those generation instructions should read like the CONTEXT.md info — a small chip, not a wall of prompt text — and open in the right panel when clicked.

Changes

  • Wrap the canvas generation boilerplate in a <canvas_generation_instructions> element; the user's instruction now leads the visible message.
  • UserMessage collapses that element into a clickable "Canvas instructions" chip (mirroring the channel CONTEXT.md chip). Clicking it opens a read-only snapshot in the right-side split via a new canvas-instructions tab. The block is always stripped so raw XML never leaks; the chip is gated behind project-bluebird.
  • Generalized the core openContextInSplit transform into openReadonlyTabInSplit so both chips share it.

How did you test this?

  • @posthog/core + @posthog/ui typecheck clean.
  • pnpm vitest run — new canvasInstructions + freeformPrompt tests and updated UserMessage tests pass (10), plus existing core panel tests (6).
  • Biome clean.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog Code

…e chip

The freeform canvas generation prompt dumped its authoring contract +
publishing/data boilerplate inline, so the user message showed all of it as
plain text. Wrap that boilerplate in a `<canvas_generation_instructions>`
element and collapse it into a clickable chip (mirroring the channel CONTEXT.md
treatment); clicking it opens a read-only snapshot in the right-side split.

The user's own instruction now leads the visible message. The block is always
stripped so the raw XML never leaks to flag-off viewers; the chip itself is
gated behind project-bluebird.

Generated-By: PostHog Code
Task-Id: 933900d5-c8d2-449e-8ea9-79861613a103
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 23121f5.

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "feat(canvas): collapse canvas generation..." | Re-trigger Greptile

Comment on lines +24 to +25
const match = CANVAS_INSTRUCTIONS_REGEX.exec(content);
if (match?.index === undefined) return null;

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 optional-chain guard match?.index === undefined is testing for two conditions simultaneously — match being null and match.index being undefined — but index is never undefined on a non-null RegExpExecArray. A plain if (!match) is equivalent and clearer.

Suggested change
const match = CANVAS_INSTRUCTIONS_REGEX.exec(content);
if (match?.index === undefined) return null;
const match = CANVAS_INSTRUCTIONS_REGEX.exec(content);
if (!match) return null;

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 +13 to +31
it("extracts the body and strips the element from the text", () => {
const content =
"What the user wants:\nadd a retention chart\n\n<canvas_generation_instructions>\nauthoring contract here\n</canvas_generation_instructions>";
const result = extractCanvasInstructions(content);
expect(result).not.toBeNull();
expect(result?.body).toBe("authoring contract here");
expect(result?.stripped).toBe(
"What the user wants:\nadd a retention chart",
);
expect(hasCanvasInstructions(content)).toBe(true);
});

it("strips the element even when it is the only content", () => {
const result = extractCanvasInstructions(
"<canvas_generation_instructions>\nbody\n</canvas_generation_instructions>",
);
expect(result?.body).toBe("body");
expect(result?.stripped).toBe("");
});

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 two extraction tests share exactly the same assertion shape (input → body + stripped). Prefer a parameterized it.each table per the repo's simplicity rules — OnceAndOnlyOnce for the test structure, and easier to extend with edge cases.

Suggested change
it("extracts the body and strips the element from the text", () => {
const content =
"What the user wants:\nadd a retention chart\n\n<canvas_generation_instructions>\nauthoring contract here\n</canvas_generation_instructions>";
const result = extractCanvasInstructions(content);
expect(result).not.toBeNull();
expect(result?.body).toBe("authoring contract here");
expect(result?.stripped).toBe(
"What the user wants:\nadd a retention chart",
);
expect(hasCanvasInstructions(content)).toBe(true);
});
it("strips the element even when it is the only content", () => {
const result = extractCanvasInstructions(
"<canvas_generation_instructions>\nbody\n</canvas_generation_instructions>",
);
expect(result?.body).toBe("body");
expect(result?.stripped).toBe("");
});
it.each([
{
label: "extracts the body and strips the element from the text",
content:
"What the user wants:\nadd a retention chart\n\n<canvas_generation_instructions>\nauthoring contract here\n</canvas_generation_instructions>",
expectedBody: "authoring contract here",
expectedStripped: "What the user wants:\nadd a retention chart",
},
{
label: "strips the element even when it is the only content",
content:
"<canvas_generation_instructions>\nbody\n</canvas_generation_instructions>",
expectedBody: "body",
expectedStripped: "",
},
])("$label", ({ content, expectedBody, expectedStripped }) => {
const result = extractCanvasInstructions(content);
expect(result).not.toBeNull();
expect(result?.body).toBe(expectedBody);
expect(result?.stripped).toBe(expectedStripped);
expect(hasCanvasInstructions(content)).toBe(true);
});

Context Used: Do not attempt to comment on incorrect alphabetica... (source)

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!

…he prompt

The instructions block opened with "Build a freeform React canvas …", which
reads as a self-contained task and could lead the agent to under-weight the
bare user instruction that now leads the message. Add a pointer in the header
(inside the tag, so it stays hidden behind the chip) tying it back to the
user's request.

Generated-By: PostHog Code
Task-Id: 933900d5-c8d2-449e-8ea9-79861613a103
@raquelmsmith raquelmsmith added the Stamphog This will request an autostamp by stamphog on small changes label Jun 23, 2026
@raquelmsmith raquelmsmith marked this pull request as ready for review June 23, 2026 23:41

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean UI feature that follows the existing channel-context chip pattern. The rename of openContextInSplit → openReadonlyTabInSplit is a safe internal refactor. The bot's inline comment about match?.index === undefined vs !match is a stylistic note — both are functionally equivalent since RegExpExecArray.index is always a number. Tests are included, the feature is flag-gated, and no data models or API contracts are broken.

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff (1)

  1. packages/ui/src/features/panels/panelLayoutStore.ts, line 189-203 (link)

    P1 Stale tab body on second canvas-generation message

    openReadonlyTabInSplit ignores the new data when the tab already exists — it only updates activeTabId (see panelLayoutTransforms.ts lines 246–258). Because the tab ID is always the fixed string "canvas-instructions", a second canvas-generation turn in the same task (e.g., the common create → edit flow) will reactivate the panel but display the first message's instructions. The second chip appears to work correctly, but the body shown is stale and may include the wrong header ("Build a…" vs "Edit a…") and an absent [Current code] block.

    The openChannelContextInSplit sibling avoids this by keying the tab ID on the channel name (context-${channelName}). A message-specific discriminator (e.g., a hash of the body or a sequential index) applied the same way here would keep each chip showing the instructions that were actually sent.

Reviews (2): Last reviewed commit: "feat(canvas): point the agent to the use..." | Re-trigger Greptile

@raquelmsmith raquelmsmith merged commit 6095bc2 into main Jun 23, 2026
30 checks passed
@raquelmsmith raquelmsmith deleted the posthog-code/canvas-instructions-chip branch June 23, 2026 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant