feat(canvas): add AI gateway built-in template#2773
Draft
richardsolomou wants to merge 1 commit into
Draft
Conversation
Ports the Cloud AI gateway usage page (PostHog/posthog#64511) to a canvas as a declarative built-in, mirroring the web-analytics template. The board reuses existing catalog components and the state.queries refresh machinery, so the only host wiring is enrolling "ai-gateway" in DATA_TEMPLATES for the date picker and toolbar refresh. Generated-By: PostHog Code Task-Id: 1e07b596-b3fb-4d08-a836-282ef28620f0
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/ui/src/features/canvas/components/WebsiteLayout.tsx:45
**Parallel registry that can drift from `BUILT_INS`**
`DATA_TEMPLATES` is a second, hand-maintained list that must stay in sync with `canvasTemplates.ts`'s `BUILT_INS`. The PR's own `AI_GATEWAY.md` notes this: "That's a parallel list to the core template registry — worth collapsing into a `dataTemplate` flag on the template record so the two can't drift." A future data-driven template will require the same two-file update in different packages, and there is nothing to catch it if one side is missed. Adding a `dataTemplate?: boolean` field to `BuiltInTemplate` and deriving this list from the registry would remove the duplication.
### Issue 2 of 3
packages/core/src/canvas/canvasTemplates.test.ts:44-50
The "connect snippets" test bundles four independent `toContain` assertions in one non-parameterised `it`. The formula test just above it already uses `it.each` for exactly this pattern, and the team preference is parameterised tests throughout. Applying the same shape here keeps the test suite consistent and gives a descriptive failure message per assertion.
```suggestion
it.each([
["OpenAI base URL", "baseURL: '<gateway base URL>/v1'"],
["Anthropic SDK import", "@anthropic-ai/sdk"],
["provider state path", '"$state": "/provider"'],
["language state path", '"$state": "/language"'],
])("bakes the %s into the connect section", (_name, snippet) => {
expect(aiGateway?.systemPrompt).toContain(snippet);
});
```
### Issue 3 of 3
packages/core/src/canvas/canvasTemplates.ts:150-155
**Fragile string surgery on `GATEWAY_WHERE`**
The empty-state probe query is constructed by calling `.replace()` twice on `GATEWAY_WHERE`. If the constant is ever restructured (e.g. the placeholder renamed, or the `AND timestamp < {date_to}` clause moved), the replacement silently produces wrong SQL with no compile-time or test-time signal. No existing test asserts the assembled empty-state query text. A named constant — `const GATEWAY_EMPTY_STATE_WHERE = \`${GATEWAY_BASE_FILTER} AND timestamp >= now() - INTERVAL 30 DAY\`` — next to `GATEWAY_WHERE` would be self-documenting and immune to structural drift.
Reviews (1): Last reviewed commit: "feat(canvas): add AI gateway built-in te..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Templates whose canvases carry the data toolbar (Filter + date range + | ||
| // refresh) — the ones with refreshable, time-scoped queries. | ||
| const DATA_TEMPLATES = ["dashboard", "web-analytics"]; | ||
| const DATA_TEMPLATES = ["dashboard", "web-analytics", "ai-gateway"]; |
Contributor
There was a problem hiding this comment.
BUILT_INS
DATA_TEMPLATES is a second, hand-maintained list that must stay in sync with canvasTemplates.ts's BUILT_INS. The PR's own AI_GATEWAY.md notes this: "That's a parallel list to the core template registry — worth collapsing into a dataTemplate flag on the template record so the two can't drift." A future data-driven template will require the same two-file update in different packages, and there is nothing to catch it if one side is missed. Adding a dataTemplate?: boolean field to BuiltInTemplate and deriving this list from the registry would remove the duplication.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/canvas/components/WebsiteLayout.tsx
Line: 45
Comment:
**Parallel registry that can drift from `BUILT_INS`**
`DATA_TEMPLATES` is a second, hand-maintained list that must stay in sync with `canvasTemplates.ts`'s `BUILT_INS`. The PR's own `AI_GATEWAY.md` notes this: "That's a parallel list to the core template registry — worth collapsing into a `dataTemplate` flag on the template record so the two can't drift." A future data-driven template will require the same two-file update in different packages, and there is nothing to catch it if one side is missed. Adding a `dataTemplate?: boolean` field to `BuiltInTemplate` and deriving this list from the registry would remove the duplication.
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
+44
to
+50
| it("includes the connect snippets and the declarative provider/language switch", () => { | ||
| const prompt = aiGateway?.systemPrompt ?? ""; | ||
| expect(prompt).toContain("baseURL: '<gateway base URL>/v1'"); // OpenAI | ||
| expect(prompt).toContain("@anthropic-ai/sdk"); // Anthropic | ||
| expect(prompt).toContain('"$state": "/provider"'); | ||
| expect(prompt).toContain('"$state": "/language"'); | ||
| }); |
Contributor
There was a problem hiding this comment.
toContain assertions in one non-parameterised it. The formula test just above it already uses it.each for exactly this pattern, and the team preference is parameterised tests throughout. Applying the same shape here keeps the test suite consistent and gives a descriptive failure message per assertion.
Suggested change
| it("includes the connect snippets and the declarative provider/language switch", () => { | |
| const prompt = aiGateway?.systemPrompt ?? ""; | |
| expect(prompt).toContain("baseURL: '<gateway base URL>/v1'"); // OpenAI | |
| expect(prompt).toContain("@anthropic-ai/sdk"); // Anthropic | |
| expect(prompt).toContain('"$state": "/provider"'); | |
| expect(prompt).toContain('"$state": "/language"'); | |
| }); | |
| it.each([ | |
| ["OpenAI base URL", "baseURL: '<gateway base URL>/v1'"], | |
| ["Anthropic SDK import", "@anthropic-ai/sdk"], | |
| ["provider state path", '"$state": "/provider"'], | |
| ["language state path", '"$state": "/language"'], | |
| ])("bakes the %s into the connect section", (_name, snippet) => { | |
| expect(aiGateway?.systemPrompt).toContain(snippet); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/canvas/canvasTemplates.test.ts
Line: 44-50
Comment:
The "connect snippets" test bundles four independent `toContain` assertions in one non-parameterised `it`. The formula test just above it already uses `it.each` for exactly this pattern, and the team preference is parameterised tests throughout. Applying the same shape here keeps the test suite consistent and gives a descriptive failure message per assertion.
```suggestion
it.each([
["OpenAI base URL", "baseURL: '<gateway base URL>/v1'"],
["Anthropic SDK import", "@anthropic-ai/sdk"],
["provider state path", '"$state": "/provider"'],
["language state path", '"$state": "/language"'],
])("bakes the %s into the connect section", (_name, snippet) => {
expect(aiGateway?.systemPrompt).toContain(snippet);
});
```
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
+150
to
+155
| "EMPTY STATE: before building, run `SELECT count() FROM events WHERE " + | ||
| GATEWAY_WHERE.replace("{date_from}", "now() - INTERVAL 30 DAY").replace( | ||
| " AND timestamp < {date_to}", | ||
| "", | ||
| ) + | ||
| '` via the MCP tools. If it returns 0 (no gateway usage in the window), do NOT build the Usage or By model sections (a zeroed-out board reads as broken). Instead emit ONLY: the h1 title + muted subtitle, a `Hero` (tone accent) titled "No gateway usage yet" whose subtitle is "One endpoint for every major LLM, billed at cost — no markup on tokens. Point your app at the gateway and PostHog tracks its usage, cost, and spend for you. Any project secret key with the llm_gateway:read scope can call it.", and the full Connect your app section.', |
Contributor
There was a problem hiding this comment.
GATEWAY_WHERE
The empty-state probe query is constructed by calling .replace() twice on GATEWAY_WHERE. If the constant is ever restructured (e.g. the placeholder renamed, or the AND timestamp < {date_to} clause moved), the replacement silently produces wrong SQL with no compile-time or test-time signal. No existing test asserts the assembled empty-state query text. A named constant — const GATEWAY_EMPTY_STATE_WHERE = \${GATEWAY_BASE_FILTER} AND timestamp >= now() - INTERVAL 30 DAY`— next toGATEWAY_WHERE` would be self-documenting and immune to structural drift.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/canvas/canvasTemplates.ts
Line: 150-155
Comment:
**Fragile string surgery on `GATEWAY_WHERE`**
The empty-state probe query is constructed by calling `.replace()` twice on `GATEWAY_WHERE`. If the constant is ever restructured (e.g. the placeholder renamed, or the `AND timestamp < {date_to}` clause moved), the replacement silently produces wrong SQL with no compile-time or test-time signal. No existing test asserts the assembled empty-state query text. A named constant — `const GATEWAY_EMPTY_STATE_WHERE = \`${GATEWAY_BASE_FILTER} AND timestamp >= now() - INTERVAL 30 DAY\`` — next to `GATEWAY_WHERE` would be self-documenting and immune to structural drift.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
New agents-platform frontend work belongs in PostHog Code, not the Cloud frontend. The AI gateway usage page shipped as a Cloud kea scene (PostHog/posthog#64511); this is the canvas-native equivalent.
Changes
ai-gatewaydeclarative built-in (KPI Stats, spend-per-day chart, by-model table, connect-snippet switch, empty-state Hero), mirroring the web-analytics template.ai-gatewayinDATA_TEMPLATESso the board gets the date picker and toolbar refresh.<placeholder>(Code has no preflight to source it); rationale and open question inAI_GATEWAY.md.How did you test this?
vitest run src/canvas/(6 files, 53 tests), core typecheck, biome lint, all green. NewcanvasTemplates.test.tsasserts the prompt bakes the exact filter, formulas, and date placeholders. Have not driven the live agent.Automatic notifications
Created with PostHog Code