feat(dashboards): add MCP tools to create and edit text tiles#59933
Merged
Conversation
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
MCP UI Apps size report
|
Contributor
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
products/dashboards/backend/api/dashboard.py:220-227
The `body` field in `UpdateTextTileRequestSerializer` accepts both `null` and empty string (`allow_null=True, allow_blank=True`), which means a caller can PATCH `body: ""` or `body: null` and the handler will set `text.body` to `""` or `None`. Since `Text.body` is `null=True, blank=True` in the model, the write will succeed — silently clearing the tile's content. The create endpoint rejects this with `allow_blank=False`; the update should do the same.
```suggestion
body = serializers.CharField(
max_length=4000,
required=False,
allow_blank=False,
help_text="New markdown body for the text tile. Omit to leave the body unchanged. Max 4000 characters.",
error_messages={"max_length": "Text body cannot exceed 4000 characters"},
)
```
### Issue 2 of 2
posthog/api/test/dashboards/test_dashboard.py:3280-3310
**Prefer parameterised tests for related rejection/404 cases**
`test_create_text_tile_rejects_empty_body` and `test_create_text_tile_rejects_body_over_4000_chars` both POST to the same endpoint and assert the same HTTP 400, differing only in the `body` value — a natural fit for a single `@pytest.mark.parametrize` / `subTest` loop. The same applies to `test_update_text_tile_with_unknown_id_returns_404` and `test_update_text_tile_on_other_dashboard_returns_404`. Repo conventions prefer parameterised tests over duplicated method bodies for scenarios that vary only in inputs.
Reviews (1): Last reviewed commit: "feat(dashboards): add create/update text..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+220
to
+227
| body = serializers.CharField( | ||
| max_length=4000, | ||
| required=False, | ||
| allow_null=True, | ||
| allow_blank=True, | ||
| help_text="New markdown body for the text tile. Omit to leave the body unchanged. Max 4000 characters.", | ||
| error_messages={"max_length": "Text body cannot exceed 4000 characters"}, | ||
| ) |
Contributor
There was a problem hiding this comment.
body field in UpdateTextTileRequestSerializer accepts both null and empty string (allow_null=True, allow_blank=True), which means a caller can PATCH body: "" or body: null and the handler will set text.body to "" or None. Since Text.body is null=True, blank=True in the model, the write will succeed — silently clearing the tile's content. The create endpoint rejects this with allow_blank=False; the update should do the same.
Suggested change
| body = serializers.CharField( | |
| max_length=4000, | |
| required=False, | |
| allow_null=True, | |
| allow_blank=True, | |
| help_text="New markdown body for the text tile. Omit to leave the body unchanged. Max 4000 characters.", | |
| error_messages={"max_length": "Text body cannot exceed 4000 characters"}, | |
| ) | |
| body = serializers.CharField( | |
| max_length=4000, | |
| required=False, | |
| allow_blank=False, | |
| help_text="New markdown body for the text tile. Omit to leave the body unchanged. Max 4000 characters.", | |
| error_messages={"max_length": "Text body cannot exceed 4000 characters"}, | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/dashboards/backend/api/dashboard.py
Line: 220-227
Comment:
The `body` field in `UpdateTextTileRequestSerializer` accepts both `null` and empty string (`allow_null=True, allow_blank=True`), which means a caller can PATCH `body: ""` or `body: null` and the handler will set `text.body` to `""` or `None`. Since `Text.body` is `null=True, blank=True` in the model, the write will succeed — silently clearing the tile's content. The create endpoint rejects this with `allow_blank=False`; the update should do the same.
```suggestion
body = serializers.CharField(
max_length=4000,
required=False,
allow_blank=False,
help_text="New markdown body for the text tile. Omit to leave the body unchanged. Max 4000 characters.",
error_messages={"max_length": "Text body cannot exceed 4000 characters"},
)
```
How can I resolve this? If you propose a fix, please make it concise.
Member
Author
There was a problem hiding this comment.
Already addressed — body is now allow_null=False, allow_blank=False, min_length=1 in UpdateTextTileRequestSerializer (products/dashboards/backend/api/dashboard.py:233-244), and test_update_text_tile_rejects_empty_or_null_body is parameterized over both None and "".
Comment on lines
+3280
to
+3310
| ) | ||
| self.assertEqual(response.status_code, status.HTTP_201_CREATED) | ||
| self.assertEqual(response.json()["text"]["body"], "Just a divider") | ||
|
|
||
| def test_create_text_tile_rejects_empty_body(self): | ||
| dashboard = Dashboard.objects.create(team=self.team, name="Test Dashboard") | ||
|
|
||
| response = self.client.post( | ||
| f"/api/environments/{self.team.pk}/dashboards/{dashboard.pk}/create_text_tile/", | ||
| {"body": ""}, | ||
| content_type="application/json", | ||
| ) | ||
| self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| def test_create_text_tile_rejects_body_over_4000_chars(self): | ||
| dashboard = Dashboard.objects.create(team=self.team, name="Test Dashboard") | ||
|
|
||
| response = self.client.post( | ||
| f"/api/environments/{self.team.pk}/dashboards/{dashboard.pk}/create_text_tile/", | ||
| {"body": "x" * 4001}, | ||
| content_type="application/json", | ||
| ) | ||
| self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| def test_create_text_tile_on_deleted_dashboard_returns_404(self): | ||
| dashboard = Dashboard.objects.create(team=self.team, name="Test Dashboard", deleted=True) | ||
|
|
||
| response = self.client.post( | ||
| f"/api/environments/{self.team.pk}/dashboards/{dashboard.pk}/create_text_tile/", | ||
| {"body": "Some text"}, | ||
| content_type="application/json", |
Contributor
There was a problem hiding this comment.
test_create_text_tile_rejects_empty_body and test_create_text_tile_rejects_body_over_4000_chars both POST to the same endpoint and assert the same HTTP 400, differing only in the body value — a natural fit for a single @pytest.mark.parametrize / subTest loop. The same applies to test_update_text_tile_with_unknown_id_returns_404 and test_update_text_tile_on_other_dashboard_returns_404. Repo conventions prefer parameterised tests over duplicated method bodies for scenarios that vary only in inputs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/api/test/dashboards/test_dashboard.py
Line: 3280-3310
Comment:
**Prefer parameterised tests for related rejection/404 cases**
`test_create_text_tile_rejects_empty_body` and `test_create_text_tile_rejects_body_over_4000_chars` both POST to the same endpoint and assert the same HTTP 400, differing only in the `body` value — a natural fit for a single `@pytest.mark.parametrize` / `subTest` loop. The same applies to `test_update_text_tile_with_unknown_id_returns_404` and `test_update_text_tile_on_other_dashboard_returns_404`. Repo conventions prefer parameterised tests over duplicated method bodies for scenarios that vary only in inputs.
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!
Member
Author
There was a problem hiding this comment.
Done — both rejection groups now use @parameterized.expand: test_create_text_tile_rejects_invalid_body covers empty + over_max_length, and test_update_text_tile_rejects_empty_or_null_body covers null + empty. The two 404 paths share fixtures via _make_unknown_tile_id_args + subTest because parameterized.expand runs at class-construction time and can't reach DB-created fixtures.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfc5002256
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+207
to
+211
| color = serializers.CharField( | ||
| required=False, | ||
| allow_null=True, | ||
| allow_blank=True, | ||
| help_text="Optional accent color name (e.g. 'blue', 'green', 'purple', 'black').", |
There was a problem hiding this comment.
color length in text-tile request serializers
The new text-tile endpoints accept color via plain CharField without a max_length, but DashboardTile.color is limited to 400 chars at the model/database layer. A request with an oversized color will pass serializer validation and fail only during save (in both create and update paths), causing a server error instead of a clean 4xx validation response. Add max_length=400 (and matching error message) to these request fields so invalid payloads are rejected before hitting the DB.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Already addressed — both CreateTextTileRequestSerializer and UpdateTextTileRequestSerializer set max_length=400 on color with a matching error_messages entry (products/dashboards/backend/api/dashboard.py:200-207 and :251-257).
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Contributor
|
Size Change: 0 B Total Size: 80.6 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 010621b3b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const DashboardsUpdateTextTilePartialUpdateBody = /* @__PURE__ */ zod.object({ | ||
| tile_id: zod | ||
| .number() | ||
| .optional() |
There was a problem hiding this comment.
tile_id in MCP update-text-tile schema
dashboard-update-text-tile currently validates tile_id as optional in the generated Zod body schema, but the backend request serializer requires it (tile_id is mandatory and immediately dereferenced). This means MCP clients can submit requests that pass local schema validation and then fail with a 400 from the API, creating a contract mismatch for the tool. Make tile_id required in the tool input schema so invalid calls are rejected before the HTTP request.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Confirmed real — services/mcp/tests/unit/__snapshots__/tool-schemas/common/dashboard-update-text-tile.json only lists ["id"] in required, even though the serializer marks tile_id as required=True. drf-spectacular isn't propagating the requirement through to the generated MCP/Zod schema. Will fix in this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f41ac1ed53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+188
to
+189
| .string() | ||
| .max(dashboardsCreateTextTileCreateBodyBodyMax) |
There was a problem hiding this comment.
The backend serializers reject blank text bodies (allow_blank=False), but the generated MCP schema only applies .max(4000) so "" passes local validation and then fails with a 400 from the API. This creates a request-contract mismatch for the new tool and causes avoidable runtime failures when an agent/tool submits an empty body. Add a minimum length/non-empty constraint (similarly for update body) in the generated schema contract.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Already addressed — both serializers set min_length=1 with an explicit min_length error message, and the regenerated snapshots at dashboard-create-text-tile.json and dashboard-update-text-tile.json both include "minLength": 1 on the body field.
| "type": "number" | ||
| } | ||
| }, | ||
| "required": ["id"], |
Contributor
There was a problem hiding this comment.
The schema required array only includes ["id"] but is missing "tile_id". The UpdateTextTileRequestSerializer at line 222-225 in products/dashboards/backend/api/dashboard.py defines tile_id with required=True, so the schema should be "required": ["id", "tile_id"]. Without this, MCP clients will not know that tile_id is mandatory, leading to 400 Bad Request errors when agents omit it.
"required": ["id", "tile_id"]
Suggested change
| "required": ["id"], | |
| "required": ["id", "tile_id"], | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Member
Author
There was a problem hiding this comment.
Confirmed real — same finding as the codex comment on api.ts. The serializer marks tile_id as required=True but drf-spectacular isn't propagating that into the generated OpenAPI schema, so the snapshot ends up with "required": ["id"] instead of ["id", "tile_id"]. Will fix the codegen in this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea6ad61bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+307
to
+309
| tile_id: zod | ||
| .number() | ||
| .optional() |
There was a problem hiding this comment.
The backend requires tile_id for update_text_tile (UpdateTextTileRequestSerializer sets required=True), but the generated MCP Zod contract marks tile_id optional. As a result, MCP callers can pass local validation with no tile id and then deterministically fail at runtime with a 400 from the API, which is a request-contract mismatch for this new tool.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Same finding as the earlier codex comment — confirmed real, fixing the codegen so tile_id propagates through OpenAPI → Orval → Zod as required.
MattPua
approved these changes
May 28, 2026
Member
Author
|
will double check the bot comments and resolve conflicts |
Adds two new dashboard actions and matching MCP tools so AI agents can create and edit markdown text tiles (used as section headings, dividers, and annotations) without having to construct a deeply nested dashboard PATCH payload. - POST /dashboards/:id/create_text_tile/ → dashboard-create-text-tile - PATCH /dashboards/:id/update_text_tile/ → dashboard-update-text-tile Both endpoints take a small focused payload (markdown body, optional layouts, optional color) and return a single DashboardTileSerializer, avoiding the cost of returning the whole dashboard for a tile-scoped change. Generated-By: PostHog Code Task-Id: 3d805ad6-0aaf-4961-bba7-643d68f41417
Address CI failures and trusted code-review bot feedback on the text-tile MCP endpoints: - mypy: narrow `tile.text` (Text | None) in test_update_text_tile_leaves_omitted_fields_unchanged by refreshing the Text row directly instead of going through the optional FK. - Reject null/blank body on `update_text_tile` for consistency with `create_text_tile` (Greptile P1, Graphite). Update silently clearing a tile to an empty body would have been a footgun. - Add `max_length=400` to the `color` field on both request serializers to match `DashboardTile.color` and surface oversize input as a 400 rather than a 500 at save time (Codex). - Parameterize duplicate rejection / 404 test methods (Greptile P2). - Add a `subTest`-driven test covering both "unknown tile_id" and "tile belongs to a different dashboard" 404 cases. - Commit MCP tool schema snapshots for the two new tools so the snapshot-staleness check passes. Generated-By: PostHog Code Task-Id: 3d805ad6-0aaf-4961-bba7-643d68f41417
Regenerate against the latest Orval-emitted Zod after the auto-bot picked up `max_length=400` on `color` and the tightened `body` validation on `update_text_tile`. Without this the snapshot test sees a stale schema and fails. Generated-By: PostHog Code Task-Id: 3d805ad6-0aaf-4961-bba7-643d68f41417
Address Codex P2 feedback on the text-tile MCP tools:
- enrich_url: use {params.id} (dashboard id from URL path) instead of
{id} from the response. The new endpoints return DashboardTile, so
result.id was the tile id — the generated _posthogUrl pointed at the
wrong dashboard URL.
- Enforce non-empty body in Zod by adding min_length=1 to both create
and update body fields. The backend already rejected blank with
allow_blank=False, but Zod only had max(4000), so empty strings would
pass local validation and round-trip to a 400.
- Drop the misleading "use dashboard-update with deleted: true" hint
from dashboard-update-text-tile's description. The MCP dashboard-update
contract doesn't expose tiles, so that path is a dead end. Deletion
via MCP needs a separate tool, out of scope for this PR.
The hand edits to services/mcp/src/{api,generated,tools/generated}/* and
products/dashboards/frontend/generated/* mirror what the codegen bot
will produce on rerun — committing them keeps CI green in the gap before
the auto-regen lands.
Generated-By: PostHog Code
Task-Id: 3d805ad6-0aaf-4961-bba7-643d68f41417
…ed in the MCP schema drf-spectacular treats PATCH actions as partial-update and marks all body fields optional in the generated OpenAPI schema, dropping tile_id from `required` even though the serializer sets `required=True`. MCP/Zod callers were not told tile_id was mandatory and hit 400s instead. Match the create_text_tile / copy_tile / reorder_tiles pattern and use POST.
andrewm4894
force-pushed
the
posthog-code/dashboard-text-tile-mcp-tools
branch
from
3f00546 to
36bfe75
Compare
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
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
2 participants
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
PostHog dashboards support text tiles (markdown blocks used as section headings, dividers, and annotations) but the MCP surface only exposed insight-tile workflows. Agents had no way to add structure to a dashboard they were building.
Today, the only path to create or update a text tile via the API is a deeply nested
PATCH /dashboards/:id/payload with atiles: [{ text: { body: ... }, ... }]array. That shape is awkward to expose to an LLM throughdashboard-update, both because the schema is complex and because the same tool then has to multiplex tile mutations against dashboard-level edits.Refs #58307
Changes
Adds two dedicated viewset actions on
DashboardsViewSetand matching MCP tool entries:POST /dashboards/:id/create_text_tile/→dashboard-create-text-tileBody:
{ body: string, layouts?: { sm?, xs? }, color? }. Creates aTextrow and aDashboardTilereferencing it, scoped to the dashboard's team. Returns aDashboardTileSerializer(tile-scoped, not the whole dashboard).PATCH /dashboards/:id/update_text_tile/→dashboard-update-text-tileBody:
{ tile_id: int, body?, layouts?, color? }. Updates only the fields that are passed (omitted fields are preserved), and bumpslast_modified_by/last_modified_at. Refuses non-text tiles with a 400.Both actions are guarded by
dashboard:writescope, checkcan_editon the dashboard, return 404 for deleted dashboards, and run the text + tile writes inside a single transaction. Returning a single tile (rather than the whole dashboard) keeps the LLM context footprint small.The new request serializers are typed end-to-end —
TileLayoutBoxSerializerandTileLayoutsSerializergive the generated OpenAPI / Zod / MCP schemas a typedlayoutsshape instead of aJSONField.Deletion is intentionally not a new tool:
dashboard-updatealready handles{ tiles: [{ id, deleted: true }] }, so a separate tool would be redundant surface area.How did you test this code?
I'm an agent (Claude Code via the PostHog Code agent). I ran:
ruff checkandruff formaton the changed Python files — cleanproducts/dashboards/mcp/tools.yaml— valid, both new tool entries resolve to the expected operation IDs (dashboards_create_text_tile_create,dashboards_update_text_tile_partial_update)Added new unit tests under
posthog/api/test/dashboards/test_dashboard.py:test_create_text_tile_adds_markdown_tile_to_dashboardtest_create_text_tile_without_layouts_uses_defaulttest_create_text_tile_rejects_empty_bodytest_create_text_tile_rejects_body_over_4000_charstest_create_text_tile_on_deleted_dashboard_returns_404test_update_text_tile_updates_body_and_layouttest_update_text_tile_leaves_omitted_fields_unchangedtest_update_text_tile_rejects_insight_tiletest_update_text_tile_with_unknown_id_returns_404test_update_text_tile_on_other_dashboard_returns_404I did not run the Django/pytest suite locally (the cloud agent environment didn't have a working Python venv pinned to 3.12.12); CI will run the full backend suite plus
build:openapito verify the generated MCP/Zod schemas.Publish to changelog?
Yes — agents can now structure dashboards they create with markdown section tiles via the MCP.
Docs update
No new user-facing docs needed; the MCP tool descriptions are self-documenting and inherited by the generated tool list.
🤖 Agent context
Authored by Claude (PostHog Code agent).
Design decisions:
dashboard-updateparam overrides. The existingdashboards_partial_updatepath supports text-tile mutations, but exposing the nestedtiles[].text.bodyshape to an LLM viaparam_overrideswould have required hand-writing a Zod schema. Two small, single-purpose endpoints match the existing tile-action precedent (copy_tile,move_tile,reorder_tiles) and yield clean, atomic MCP tools.copy_tile,reorder_tiles) return the entireDashboardSerializer. For text-tile changes that's overkill — returningDashboardTileSerializerkeeps the response small for the LLM and the human alike.layoutsserializer.JSONFieldwould have producedRecord<string, unknown>in the generated types. The typedTileLayoutBoxSerializer/TileLayoutsSerializergive callers a real schema with field-levelhelp_text.dashboard-updatewith{tiles: [{id, deleted: true}]}.update_text_tilerejects insight tiles with a 400 rather than silently doing nothing, since a wrongtile_idis far more likely an agent mistake worth surfacing than a desired no-op.Created with PostHog Code