Skip to content

fix(stories): persist MCP query templates - #1419

Open
btalx wants to merge 1 commit into
getnao:mainfrom
btalx:benoit/eng-9200-story-query-persistence
Open

fix(stories): persist MCP query templates#1419
btalx wants to merge 1 commit into
getnao:mainfrom
btalx:benoit/eng-9200-story-query-persistence

Conversation

@btalx

@btalx btalx commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix story filters for charts created through the MCP context-layer flow.

MCP execute_sql previously persisted only result rows. When create_story or update_story attached those rows to a chat, it pinned an empty SQL string, so the story could render cached data but could not validate or re-execute filter templates.

Changes

  • persist sql_query and database_id with MCP query data
  • backfill retained query definitions from MCP call logs in both PostgreSQL and SQLite migrations
  • pin the persisted definition with cached results when a story is attached to a chat
  • preserve the existing data-only behavior for chat-created queries
  • add regression coverage for both paths

Verification

  • targeted story/filter tests: 6 passed
  • full backend/frontend/shared lint: passed
  • formatting check: passed
  • PostgreSQL and SQLite migration consistency checks: passed
  • fresh SQLite migration: passed
  • complete pre-commit hook, including Python CLI checks: passed

Linear: https://linear.app/lucis-life/issue/ENG-9200/nao-story-filters-render-but-do-not-re-execute-persisted-chart-queries

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

2 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/backend/migrations-postgres/0064_persist_mcp_query_definitions.sql">

<violation number="1" location="apps/backend/migrations-postgres/0064_persist_mcp_query_definitions.sql:8">
P2: When an expired MCP query is reattached before this migration runs, `call_log_id` may point to the attaching tool rather than the original `execute_sql` call, so this join skips the row and leaves its SQL definition null. Backfill by matching the original query ID from the execute_sql log output, or preserve the original execute_sql association during cache refresh.</violation>
</file>

<file name="apps/backend/src/mcp/tools/context-layer.ts">

<violation number="1" location="apps/backend/src/mcp/tools/context-layer.ts:298">
P3: `resolveMcpQueryDefinitions` issues one separate `getMcpQueryData` DB query per query id found in the story code, running for every chat attachment. Because all rows share the same project/user/expiry predicates, batch them into a single query using `inArray(...queryId)` instead of an N+1 fan-out.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

"sql_query" = "mcp_call_log"."tool_input"->>'sql_query',
"database_id" = "mcp_call_log"."tool_input"->>'database_id'
FROM "mcp_call_log"
WHERE "mcp_call_log"."id" = "mcp_query_data"."call_log_id"

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: When an expired MCP query is reattached before this migration runs, call_log_id may point to the attaching tool rather than the original execute_sql call, so this join skips the row and leaves its SQL definition null. Backfill by matching the original query ID from the execute_sql log output, or preserve the original execute_sql association during cache refresh.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/migrations-postgres/0064_persist_mcp_query_definitions.sql, line 8:

<comment>When an expired MCP query is reattached before this migration runs, `call_log_id` may point to the attaching tool rather than the original `execute_sql` call, so this join skips the row and leaves its SQL definition null. Backfill by matching the original query ID from the execute_sql log output, or preserve the original execute_sql association during cache refresh.</comment>

<file context>
@@ -0,0 +1,9 @@
+	"sql_query" = "mcp_call_log"."tool_input"->>'sql_query',
+	"database_id" = "mcp_call_log"."tool_input"->>'database_id'
+FROM "mcp_call_log"
+WHERE "mcp_call_log"."id" = "mcp_query_data"."call_log_id"
+	AND "mcp_call_log"."tool_name" = 'execute_sql';
</file context>


async function resolveMcpQueryDefinitions(code: string, ctx: McpContext) {
const queryIds = [...extractQueryIds(code)];
const rows = await Promise.all(

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.

P3: resolveMcpQueryDefinitions issues one separate getMcpQueryData DB query per query id found in the story code, running for every chat attachment. Because all rows share the same project/user/expiry predicates, batch them into a single query using inArray(...queryId) instead of an N+1 fan-out.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/mcp/tools/context-layer.ts, line 298:

<comment>`resolveMcpQueryDefinitions` issues one separate `getMcpQueryData` DB query per query id found in the story code, running for every chat attachment. Because all rows share the same project/user/expiry predicates, batch them into a single query using `inArray(...queryId)` instead of an N+1 fan-out.</comment>

<file context>
@@ -285,10 +288,27 @@ async function cacheStoryQueryData(
 
+async function resolveMcpQueryDefinitions(code: string, ctx: McpContext) {
+	const queryIds = [...extractQueryIds(code)];
+	const rows = await Promise.all(
+		queryIds.map((queryId) => getMcpQueryData(queryId, ctx.projectId, { userId: ctx.userId })),
+	);
</file context>

@btalx btalx closed this Aug 19, 2026
@Bl3f Bl3f reopened this Aug 19, 2026
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