platform-lite: serve the ClickHouse logs endpoint (/analytics/endpoints/logs) - #99
Open
barryroodt wants to merge 17 commits into
Open
platform-lite: serve the ClickHouse logs endpoint (/analytics/endpoints/logs)#99barryroodt wants to merge 17 commits into
barryroodt wants to merge 17 commits into
Conversation
…ts/logs)
mcp >= the #326 migration queries /v1/projects/{ref}/analytics/endpoints/logs
with ClickHouse-dialect SQL over a unified 'logs' stream; platform-lite only
served the legacy BigQuery-era logs.all, so any logs eval against a current
mcp build 404s at the fixture.
- unified 'logs' VIEW over the seeded tables (source discriminator +
log_attributes jsonb built from columns, metadata fallback)
- minimal dialect translation: log_attributes['k'] -> jsonb access (numeric
cast for status/exec-time keys), countIf -> count(*) FILTER
- read-only enforced by a postgres read-only transaction (not regex): mutating
SQL incl. data-modifying CTEs is rejected before touching fixture state
- iso_timestamp_start/end accepted but ignored (scenario seeds carry fixed
dates; the legacy route makes the same choice)
- contract test: mcp edge-function preset, countIf aggregation, runtime source
Live A/B of mcp PR#333 showed claude-sonnet-5 emitting genuine ClickHouse (countIf(toInt32OrZero(log_attributes['status']) >= 400)); the fixture rejected it and the model adapted with postgres-only SQL that the hosted ClickHouse endpoint would refuse — greening the eval by fixture-adaptation. Provide toInt32OrZero/toInt64OrZero/toUInt32OrZero (text + numeric overloads, CH 0-on-garbage semantics) so the fixture accepts the model's natural dialect. Contract test uses the verbatim model-emitted query.
Second fixture gap from the live PR#333 treatment rerun: the model nests toString() inside toInt32OrZero(). One anyelement cast function covers it; verbatim-model-SQL contract test added.
…me-semantics limitation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The PR claims data-modifying CTEs are rejected, but the existing tests called
the translator/db directly. Exercise the actual HTTP route: normal ClickHouse
query returns the {result} shape; WITH x AS (DELETE ... RETURNING *) SELECT is
rejected by the read-only transaction with fixture rows provably unchanged;
plain non-SELECT hits the 400 prefix gate.
Rodriguespn
reviewed
Jul 21, 2026
Rodriguespn
reviewed
Jul 21, 2026
Rodriguespn
requested changes
Jul 21, 2026
…face Review question on #99: why are these SQL statements defined here instead of imported? Answer, now in-code: the logs relation shape is the hosted platform's Logflare/ClickHouse contract (supabase/platform#35096, platform-internal, no npm artifact); source names track mcp's logsServiceSchema under the pinned MCP_SERVER_VERSION; the OrZero/toString family reimplements ClickHouse builtins; and the verbatim test SQL is frozen observed output on purpose (importing live definitions would make the contract tests tautological).
…t tripwire Review follow-up on #99: the earlier provenance note (and my reply) claimed nothing was importable - wrong on one count. The pinned mcp package DOES export logsServiceSchema from its /platform entrypoint; what it enumerates is the service-preset namespace, not the unified-stream source names the view discriminates on (those exist only in preset SQL strings and the query_logs description). Comment corrected, and the importable artifact is now used for what it's genuinely good for: a drift tripwire that fails loudly when a version bump changes the service enum, pointing at what to resync.
…ovenance Second thought on the tripwire: it asserted the service-PRESET enum, which is a different namespace from the view's source names, so its failure could not demonstrate view staleness. Worse, it imported the resolved devDependency (^0.8.1 -> 0.8.2 today) while the harness runs the MCP_SERVER_VERSION pin (0.8.1), so it guarded a version the fixture never exercises. The verbatim frozen preset SQL in these tests remains the honest alignment contract. The provenance comment now records the exported-schema nuance and why importing it would track the wrong artifact.
Verified against supabase/platform directly: #35096 (getLogs -> logs.all.otel unified stream, ClickHouse dialect) and #35970 (query_logs passthrough, timestamps normalized platform-side) are both OPEN, so the contract this fixture models is what mcp main is written against, not what hosted serves today. The 35970 e2e spec uses the same source vocabulary as this view (postgres_logs), which is a good consistency signal.
#35096 backs the getLogs PRESET path (logs.all.otel + CH dialect) that mcp main emits post-#326; #35970 backs the custom-SQL passthrough that the still open mcp#333 targets - main does not depend on it. Hosted serves the /analytics/endpoints/logs route today; it is the per-PR capabilities that are pending, not the route.
The header bundled query_logs into 'current mcp' while the provenance bullets below correctly note #333 is still open; both now say: current main emits the get_logs presets, the #333 branch emits query_logs, and the fixture models both.
…ate test
- 400 prefix-reject body now carries a message key: mcp's assertSuccess
parses non-2xx bodies as {message}, so the informative read-only text
was collapsing to the generic 'Failed to fetch logs' fallback (error
kept for shape consistency with the 200 SQL-error path)
- CTE-reject test pins status 200: the prefix gate's 400 message also
matches /read-only/i, so unifying the rejection paths would otherwise
leave the read-only transaction guard silently untested
- move src/management-api/debugging.test.ts -> test/clickhouse-logs.test.ts:
platform-lite tests live under test/, and the src placement collided on
basename with the existing test/debugging.test.ts
barryroodt
force-pushed
the
platform-lite-clickhouse-logs
branch
from
July 23, 2026 13:14
1970f54 to
b759ca2
Compare
Proposal 4 (review): drop the implicit numeric cast on response.status_code/status_code/execution_time_ms map access. Hosted ClickHouse map values are String, so a bare comparison like log_attributes['response.status_code'] >= 500 errors there — the fixture now errors identically instead of silently accepting SQL that would fail hosted (eval-greens-locally hazard). Models adapt by wrapping in toInt32OrZero, exactly as the frozen fixtures show; a new test pins the error friction, and the constructed query_logs-style test now wraps its comparison like a hosted-correct query must. Minors: type the read-only transaction result (cast gone); parametrize the two verbatim PR-333 fixtures with it.each; assert function_id/level values in the runtime-preset test instead of bare row count; typed Pick<> partial for the fake store; document the two unmodeled preset sources (workflow_run_logs, realtime_logs) in the view header.
barryroodt
force-pushed
the
platform-lite-clickhouse-logs
branch
from
July 23, 2026 13:30
ba53d15 to
9d164ad
Compare
- openapi.json: advertise /analytics/endpoints/logs — spliced the single generated path entry (upstream does advertise it; AnalyticsResponse ref already present) instead of taking the full regen's unrelated drift; pinned alongside logs.all in openapi.test.ts - unmodeled sources now error loudly: compileClickHouseLogsSql rejects queries naming workflow_run_logs/realtime_logs (no backing table) so a branch-action/realtime eval fails visibly instead of reading a silent 0-row result as 'no logs'; tested at translator and HTTP level - route test store: real init-free ProjectInstance in a real Map — the exact ProjectStore shape, both casts gone
barryroodt
force-pushed
the
platform-lite-clickhouse-logs
branch
from
July 23, 2026 13:34
9d164ad to
89b814d
Compare
mattrossman
reviewed
Jul 23, 2026
Collaborator
|
Surfacing these threads for awareness: (1) (2) I'm wondering how this approach of translating CH dialect for our PGLite logs backend compares with running actual ClickHouse separate from the PGLite logs backend, using their in-memory chdb and chdb-node bindings? That might get us closer feature parity w/o manual translation for each new syntax the agent tries using, though I'm not sure about the integration lift. |
storage_logs was half-modeled: the table exists and the logs VIEW serves a 'storage_logs' source (which mcp's storage preset filters on), but seedLogRow silently dropped 'storage' seeds — a storage eval would read the resulting empty result as 'no logs', the exact false-green the unmodeled-source guard exists to prevent. Add seedStorageLog (base columns; the preset selects only id/timestamp/event_message) and a verbatim storage-preset test. seedLogRow's fall-through was the same bug at the seed layer: any unknown source (typo or unsupported service) silently seeded nothing. It now throws at seed time, naming the supported sources; tested.
…o shims Two more hosted-parity closures: - Restrict /analytics/endpoints/logs to the 'logs' relation (mattrossman's review question). ENFORCEMENT is DB-level: the route transaction runs SET LOCAL ROLE logs_reader, granted SELECT only on the logs view, so postgres name resolution denies backing-table access under any spelling (edge_logs, public.edge_logs, "edge_logs"). The FROM/JOIN regex remains as best-effort message shaping pointing the model at the source-filter idiom. The legacy logs.all route sets no role and keeps table access for its BigQuery-era dialect. The CTE read-only test now uses an INSERT CTE (passes prefix gate and regex) so the transaction stays the tested last line of defense; qualified/quoted bypass spellings are pinned in tests. - Drop the numeric *OrZero overloads: ClickHouse's toInt32OrZero family takes String only, so toInt32OrZero(42) must error here as it does hosted. They existed for the translator's implicit numeric casts, which are already gone. Negative parity test added.
barryroodt
force-pushed
the
platform-lite-clickhouse-logs
branch
from
July 23, 2026 13:51
e546500 to
c3dfccf
Compare
Contributor
Author
|
chdb could be a good fit eventually, yeah, thanks for the threads. Today it'd be a second engine (legacy |
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
3 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.
What
Teaches platform-lite the ClickHouse logs endpoint that current mcp actually calls:
GET /v1/projects/{ref}/analytics/endpoints/logs, taking ClickHouse-dialect SQL over the unifiedlogsstream.Why
Since supabase/mcp#326,
get_logs(and the proposedquery_logsin supabase/mcp#333) query/analytics/endpoints/logswith ClickHouse SQL. platform-lite only served the legacy BigQuery-eralogs.all, so any logs eval against a locally built mcp 404s at the fixture. The gap is masked today because evals pin a publishedMCP_SERVER_VERSION; it bites the moment anyone points the harness at an mcp checkout (which is how we validated mcp#333).How
logsVIEW over the existing seeded tables: asourcediscriminator plus a jsonblog_attributesmap built from the flat columns, with seededmetadataas fallback. ClickHouse-shaped SQL runs against it with minimal translation.compileClickHouseLogsSql):log_attributes['k']to jsonb access (numeric cast for status/exec-time keys so>= 500comparisons work) andcountIf(...)tocount(*) FILTER (WHERE ...). A small shim family (toInt32OrZero/toInt64OrZero/toUInt32OrZero/toString) covers casts models genuinely emitted during live runs. Anything else surfaces the raw SQL error to the model, which is deliberate: the supported surface is documented and only grows from observed model output.WITH x AS (DELETE ...) SELECT), is rejected before it can touch shared fixture state.iso_timestamp_start/endare accepted but ignored, matching the legacy route: scenario seeds carry fixed dates while mcp defaults windows from the current clock, so a faithful filter would empty every scenario. Documented in-code as a known limitation; window-correctness needs relative-time seeding and a discriminating eval (follow-up).edge-functionpreset, acountIfaggregation, and the exacttoInt32OrZero(toString(...))query the model emitted), plus the runtime source and the legacy route untouched.Verification
pnpm typecheckclean,pnpm vitest run src/management-api/debugging.test.ts8/8: five translator/view contract tests plus three route-level tests at the HTTP boundary (normal ClickHouse query returns the{result}shape;WITH x AS (DELETE ... RETURNING *) SELECTis rejected by the read-only transaction with fixture rows asserted unchanged; plain non-SELECT hits the 400 prefix gate).investigate-logs-001-top-error-functionpasses against a locally built mcpmain(3/3 checks) where it previously 404'd, and against an mcp checkout of feat: add query_logs tool for custom log queries mcp#333 the model's first genuine ClickHouse aggregation succeeds end to end.Found while running an A/B validation of supabase/mcp#333 through the eval workspace; the run details are in that PR's thread.