diff --git a/odd/ledger/learnings.jsonl b/odd/ledger/learnings.jsonl index f00ada0..3e4cd07 100644 --- a/odd/ledger/learnings.jsonl +++ b/odd/ledger/learnings.jsonl @@ -38,3 +38,5 @@ {"id":"learn-20260412-0001","timestamp":"2026-04-12T00:52:00Z","summary":"Standalone Worker tools (telemetry, time) bypass orchestrate pipeline — they share oddkit_ MCP prefix but register directly in createServer with their own handler. CLI parity requires adding to TOOLS array (auto-cascades) plus explicit param threading in cli.js and server.js","trigger":"architecture","impact":"New standalone tools need 5 files touched: index.ts (Worker registration), tool-registry.js (TOOLS entry), actions.js (handler), server.js (param threading), cli.js (param threading). The TOOLS auto-derivation handles enum/listing but not param plumbing.","confidence":0.95,"sources":["workers/src/index.ts","src/core/tool-registry.js","src/core/actions.js","src/mcp/server.js","src/cli.js"],"evidence":[{"type":"artifact","ref":"PR #87 — oddkit_time implementation across 5 files"}],"candidate_targets":[],"proposed_escalation":"none"} {"id":"L39","timestamp":"2026-04-13T11:12:00Z","type":"learning","summary":"raw.githubusercontent.com URL parsing must rejoin all path segments after owner/repo to support branch names with slashes — parts[2] truncates multi-segment refs like publish/four-essays-and-skill to just publish","context":"extractBranchRef() and getZipUrl() in zip-baseline-fetcher.ts both used parts[2] which only captured the first segment of a slash-containing branch name, causing 404s on both SHA resolution and ZIP download","resolution":"Changed to parts.slice(2).join(\"/\") in both functions — minimal 2-line fix"} {"type":"D","summary":"E0008 challenge governance refactor: replaced hardcoded detectClaimType logic in runChallengeAction with four governance-driven fetch functions (discoverChallengeTypes, fetchBasePrerequisites, fetchNormativeVocabulary, fetchStakesCalibration). Voice-dump suppression invariant is load-bearing — questionTiers.length === 0 short-circuits all output. Four new caches cleared in runCleanupStorage. tsc clean. PR #100.","rationale":"Hardcoded challenge logic cannot evolve with governance articles; governance-driven extraction means challenge behavior updates when articles update, no code change required. Mirrors PR #96 encode precedent exactly.","context":"workers/src/orchestrate.ts, branch feat/e0008-challenge-governance-driven, commit aa4445c","date":"2026-04-17"} +{"date": "2026-04-24", "epoch": "E0008", "task": "feat/telemetry-semantic-names", "summary": "TypeScript bundler moduleResolution omits .js extensions on local imports in compiled output \u2014 Node.js ESM resolver requires explicit .js suffix. When compiling telemetry.ts for integration tests, all compiled .js files in the build dir must be post-processed to add .js to extensionless relative imports. Patch all files in the build dir, not just telemetry.js.", "detail": "telemetry.ts now imports KnowledgeBaseFetcher (a value import, not just a type import) from zip-baseline-fetcher.ts. The existing integration test only compiled tokenize.ts and telemetry.ts. Adding zip-baseline-fetcher.ts to the tsconfig include list is necessary but insufficient \u2014 the compiled JS has extensionless imports (./zip-baseline-fetcher, ./tracing) that Node ESM cannot resolve. Must patch all .js files in the build dir with a regex replace of from \"./foo\" -> from \"./foo.js\".", "pr": "https://github.com/klappy/oddkit/pull/137"} +{"date": "2026-04-24", "epoch": "E0008", "task": "feat/telemetry-semantic-names", "summary": "JSDoc block comments must not contain */ sequences \u2014 they terminate the comment prematurely. Patterns like blob*/double* in a JSDoc comment cause TypeScript parse errors. Use blob1..9/double1..6 or similar notation instead.", "detail": "detectRawSlotNames JSDoc had blob*/double* which the TypeScript parser reads as end-of-comment at the first */. tsc reported TS1109 (Expression expected) at line 459. The fix is trivial but the error message is cryptic \u2014 the real cause is invisible until you stare at the raw characters.", "pr": "https://github.com/klappy/oddkit/pull/137"} diff --git a/workers/src/index.ts b/workers/src/index.ts index 5c646c2..bf7476b 100644 --- a/workers/src/index.ts +++ b/workers/src/index.ts @@ -425,20 +425,31 @@ Use when: Dataset: oddkit_telemetry (Cloudflare Analytics Engine) Schema: - blob1 — event_type "mcp_request" | "tool_call" - blob2 — method JSON-RPC method (e.g. "tools/call") - blob3 — tool_name oddkit action (e.g. "orient", "search") - blob4 — consumer_label best-effort caller identity - blob5 — consumer_source how label was resolved (e.g. "user-agent") - blob6 — knowledge_base_url which knowledge base is being served - blob7 — document_uri for get calls, the klappy:// URI requested - blob8 — worker_version oddkit version string - double1 — count always 1 - double2 — duration_ms request processing time - index1 — sampling_key consumer label + event_type — "mcp_request" | "tool_call" + method — JSON-RPC method (e.g. "tools/call") + tool_name — oddkit action (e.g. "orient", "search") + consumer_label — best-effort caller identity + consumer_source — how label was resolved (e.g. "user-agent") + knowledge_base_url — which knowledge base is being served + document_uri — for get calls, the klappy:// URI requested + worker_version — oddkit version string + cache_tier — which storage tier served the index + count — always 1 (use SUM for aggregation) + duration_ms — request processing time (full wall-clock at worker edge) + bytes_in — UTF-8 byte length of the request body + bytes_out — UTF-8 byte length of the response body (0 for SSE streams) + tokens_in — cl100k_base token count of the request body + tokens_out — cl100k_base token count of the response body + index1 — sampling key (consumer label) Use SUM(_sample_interval) instead of COUNT(*) to account for Analytics Engine sampling. -Time filter example: WHERE timestamp > NOW() - INTERVAL '30' DAY`, +Time filter example: WHERE timestamp > NOW() - INTERVAL '30' DAY + +Example — tool leaderboard: + SELECT tool_name, SUM(_sample_interval) AS calls FROM oddkit_telemetry WHERE timestamp > NOW() - INTERVAL '30' DAY GROUP BY tool_name ORDER BY calls DESC LIMIT 10 + +Example — payload shape by tool: + SELECT tool_name, AVG(tokens_out) AS avg_tokens_out FROM oddkit_telemetry WHERE timestamp > NOW() - INTERVAL '7' DAY GROUP BY tool_name ORDER BY avg_tokens_out DESC`, { sql: z.string().describe("Analytics Engine SQL query against the oddkit_telemetry dataset."), }, diff --git a/workers/src/telemetry.ts b/workers/src/telemetry.ts index e6baf02..07f5413 100644 --- a/workers/src/telemetry.ts +++ b/workers/src/telemetry.ts @@ -55,6 +55,7 @@ */ import type { Env } from "./zip-baseline-fetcher"; +import { KnowledgeBaseFetcher } from "./zip-baseline-fetcher"; import type { PayloadShape } from "./tokenize"; import pkg from "../package.json"; @@ -300,6 +301,294 @@ export function recordTelemetry( } } +// ────────────────────────────────────────────────────────────────────────────── +// Semantic schema mapping (vodka-architecture compliant) +// +// Cloudflare Analytics Engine uses positional slot names (blob1..9, double1..6). +// Consumers see semantic names only. This module: +// 1. Loads the authoritative mapping from canon at runtime +// (canon/constraints/telemetry-governance.md, parsed via KnowledgeBaseFetcher) +// 2. Falls back to the hardcoded baseline below if canon is unreachable +// 3. Rewrites consumer SQL (semantic → raw) before forwarding to CF +// 4. Rewrites result column names (raw → semantic) before returning to consumer +// 5. Rejects any query containing raw blob*/double* names with a helpful error +// +// Source of truth: klappy://canon/constraints/telemetry-governance +// ────────────────────────────────────────────────────────────────────────────── + +export interface SchemaMap { + /** raw slot name → semantic name (e.g. blob1 → event_type) */ + rawToSemantic: Map; + /** semantic name → raw slot name (e.g. event_type → blob1) */ + semanticToRaw: Map; +} + +/** + * Canonical blob→semantic ordering derived from canon table + * "Structural Dimensions (Blobs)" (positional, 9 entries). + * The canon doc uses human-readable dimension names in that table, not + * machine-readable column identifiers, so positional order is the only + * reliable parse signal. These names mirror what the tool docstring exposes. + */ +const BASELINE_BLOB_SEMANTIC_NAMES = [ + "event_type", // blob1 + "method", // blob2 + "tool_name", // blob3 + "consumer_label", // blob4 + "consumer_source", // blob5 + "knowledge_base_url", // blob6 + "document_uri", // blob7 + "worker_version", // blob8 + "cache_tier", // blob9 +] as const; + +/** + * Baseline double→semantic names. Canon table "Numeric Values (Doubles)" + * encodes these with backtick-quoted identifiers (except "Count" → "count") + * which are parseable at runtime. Baseline is the safety net. + */ +const BASELINE_DOUBLE_SEMANTIC_NAMES = [ + "count", // double1 + "duration_ms", // double2 + "bytes_in", // double3 + "bytes_out", // double4 + "tokens_in", // double5 + "tokens_out", // double6 +] as const; + +/** Build a SchemaMap from ordered blob/double name arrays. Exported for unit testing. */ +export function buildSchemaMapFromArrays( + blobNames: readonly string[], + doubleNames: readonly string[], +): SchemaMap { + const rawToSemantic = new Map(); + const semanticToRaw = new Map(); + + blobNames.forEach((name, i) => { + const slot = `blob${i + 1}`; + rawToSemantic.set(slot, name); + semanticToRaw.set(name, slot); + }); + + doubleNames.forEach((name, i) => { + const slot = `double${i + 1}`; + rawToSemantic.set(slot, name); + semanticToRaw.set(name, slot); + }); + + return { rawToSemantic, semanticToRaw }; +} + +/** + * Attempt to parse semantic double names from the canon governance document. + * The "Numeric Values (Doubles)" table in canon uses backtick-quoted identifiers + * in the "Value" column (e.g. `duration_ms`, `bytes_in`). Row 1 uses "Count" + * (unquoted) which we lowercase to "count". + * + * Returns null if the section is missing or has too few rows to be trusted. + */ +function parseDoublesFromCanon(content: string): string[] | null { + // Match the Numeric Values section up to the next ## or ### heading + const sectionMatch = content.match( + /###\s+Numeric Values \(Doubles\)([\s\S]*?)(?=\n###|\n##|$)/, + ); + if (!sectionMatch) return null; + + const section = sectionMatch[1]; + + // Match table data rows: | # | Value | ... + // Value may be plain text (Count) or backtick-quoted (`duration_ms`) + const rowPattern = /^\|\s*\d+\s*\|\s*(`[^`]+`|[A-Za-z_][A-Za-z0-9_]*)\s*\|/gm; + const names: string[] = []; + let m: RegExpExecArray | null; + while ((m = rowPattern.exec(section)) !== null) { + const raw = m[1].replace(/`/g, "").trim().toLowerCase(); + names.push(raw); + } + + // Sanity: must match expected baseline count + if (names.length !== BASELINE_DOUBLE_SEMANTIC_NAMES.length) return null; + + return names; +} + +/** Module-level cache — reset between Worker isolate restarts (Cloudflare normal). */ +let cachedSchemaMap: SchemaMap | null = null; + +/** + * Return the schema map, loading from canon on first call. + * Canon-derived doubles names take precedence over baseline; blob names are + * always positional from the baseline (the canon table uses human-readable + * dimension names, not machine-readable identifiers). + */ +async function getSchemaMap(env: Env): Promise { + if (cachedSchemaMap) return cachedSchemaMap; + + let doubleNames: readonly string[] = BASELINE_DOUBLE_SEMANTIC_NAMES; + + try { + const fetcher = new KnowledgeBaseFetcher(env); + const content = await fetcher.getFile( + "canon/constraints/telemetry-governance.md", + ); + if (content) { + const parsed = parseDoublesFromCanon(content); + if (parsed) { + doubleNames = parsed; + } + } + } catch { + // Canon unreachable — fall through to baseline (vodka architecture safety net) + } + + cachedSchemaMap = buildSchemaMapFromArrays( + BASELINE_BLOB_SEMANTIC_NAMES, + doubleNames, + ); + return cachedSchemaMap; +} + +// ────────────────────────────────────────────────────────────────────────────── +// SQL query rewriting +// ────────────────────────────────────────────────────────────────────────────── + +/** Raw slot name pattern — used to detect forbidden column references. */ +const RAW_SLOT_PATTERN = /\b(blob[1-9]|double[1-9])\b/gi; + +/** + * Reject queries that contain raw slot names (blob1..9 / double1..6). + * Returns an error message string, or null if the query is clean. + * + * Single-quoted string literals are skipped so that values like + * `'https://example.com/blob1/readme'` do not trigger a false rejection. + * This matches the scoping rules used by `rewriteSqlToRaw`. SQL's + * doubled-quote escape (`''`) is respected so that escaped quotes do not + * terminate the literal prematurely. + * Exported for unit testing. + */ +export function detectRawSlotNames( + sql: string, + schemaMap: SchemaMap, +): string | null { + // Strip single-quoted string literals before scanning for raw slot names + // so filter values containing substrings like `blob1` are not flagged. + const literalPattern = /'(?:[^']|'')*'/g; + const scannable = sql.replace(literalPattern, "''"); + const matches = scannable.match(RAW_SLOT_PATTERN); + if (!matches) return null; + + const unique = [...new Set(matches.map((m) => m.toLowerCase()))]; + const suggestions = unique + .map((raw) => { + const semantic = schemaMap.rawToSemantic.get(raw); + return semantic ? `\`${raw}\` → \`${semantic}\`` : `\`${raw}\` (unmapped)`; + }) + .join(", "); + + return ( + `Raw column names are not allowed. Use semantic names instead: ${suggestions}. ` + + `See the telemetry_public tool description for the full schema.` + ); +} + +/** + * Rewrite a SQL query from semantic names to raw Analytics Engine slot names. + * Semantic names are matched on word boundaries to avoid partial replacements. + * Longer names are replaced first to prevent prefix collisions (e.g. + * knowledge_base_url vs url). + * + * A negative lookahead `(?!\s*\()` prevents rewriting identifiers that are + * immediately followed by an opening parenthesis — i.e. SQL function calls. + * This is required because the semantic name `count` collides with the SQL + * aggregate `count()`; without the guard, `count(*)` would become + * `double1(*)` and be rejected by the CF API. Column references never + * take a `(` after them, so this is safe for all semantic names. + * + * Single-quoted string literals are skipped so that values like + * `'klappy://sources/scientific-method'` are not corrupted (word boundaries + * around `-` / `/` would otherwise cause `method` to be rewritten to the raw + * slot name inside the literal). SQL's doubled-quote escape (`''`) is + * respected so that escaped quotes do not terminate the literal prematurely. + * Exported for unit testing. + */ +export function rewriteSqlToRaw(sql: string, schemaMap: SchemaMap): string { + // Sort by semantic name length descending to avoid prefix collisions + const entries = [...schemaMap.semanticToRaw.entries()].sort( + (a, b) => b[0].length - a[0].length, + ); + + const rewriteSegment = (segment: string): string => { + let out = segment; + for (const [semantic, raw] of entries) { + // \b word-boundary anchors prevent partial matches inside longer identifiers. + // Negative lookahead (?!\s*\() skips function-call positions (e.g. count(*)). + const pattern = new RegExp(`\\b${semantic}\\b(?!\\s*\\()`, "g"); + out = out.replace(pattern, raw); + } + return out; + }; + + // Split SQL into alternating non-literal and single-quoted literal segments. + // Only non-literal segments are subject to rewriting, so user-supplied + // filter values passed as string literals are preserved verbatim. + const literalPattern = /'(?:[^']|'')*'/g; + let rewritten = ""; + let lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = literalPattern.exec(sql)) !== null) { + rewritten += rewriteSegment(sql.slice(lastIndex, match.index)); + rewritten += match[0]; + lastIndex = match.index + match[0].length; + } + rewritten += rewriteSegment(sql.slice(lastIndex)); + return rewritten; +} + +/** + * Rewrite result column names from raw slot names back to semantic names. + * Operates on the `meta` array (CF Analytics Engine response format) and + * rewrites the corresponding keys in each `data` row. + * Exported for unit testing. + */ +export function rewriteResultToSemantic( + result: unknown, + schemaMap: SchemaMap, +): unknown { + if (typeof result !== "object" || result === null) return result; + + const r = result as Record; + if (!Array.isArray(r.meta)) return result; + + type MetaCol = { name: string; type: string }; + const oldMeta = r.meta as MetaCol[]; + + // Build a remapping from old column name → semantic name (only for slots) + const colRemap = new Map(); + const newMeta: MetaCol[] = oldMeta.map((col) => { + const semantic = schemaMap.rawToSemantic.get(col.name); + if (semantic && semantic !== col.name) { + colRemap.set(col.name, semantic); + return { ...col, name: semantic }; + } + return col; + }); + + if (colRemap.size === 0) return result; // nothing to rename + + // Rewrite data rows + const newData = Array.isArray(r.data) + ? (r.data as Array>).map((row) => { + const newRow: Record = {}; + for (const [key, val] of Object.entries(row)) { + newRow[colRemap.get(key) ?? key] = val; + } + return newRow; + }) + : r.data; + + return { ...r, meta: newMeta, data: newData }; +} + // ────────────────────────────────────────────────────────────────────────────── // Analytics Engine SQL query // ────────────────────────────────────────────────────────────────────────────── @@ -345,10 +634,17 @@ function validateTelemetryQuery(query: string): string | null { } /** - * Query Analytics Engine SQL API. + * Query Analytics Engine SQL API with semantic-name rewriting. * Used by telemetry_public tool. * Requires CF_ACCOUNT_ID and CF_API_TOKEN env vars. * Only permits SELECT queries against the oddkit_telemetry dataset. + * + * Semantic-name contract: + * - Consumers write SQL using semantic names (event_type, tool_name, etc.) + * - Raw slot names (blob1, double2, etc.) are rejected with a helpful error + * - The schema mapping is loaded from canon at runtime; the hardcoded baseline + * is the safety net when canon is unreachable (vodka architecture) + * - Result columns are renamed from raw slots back to semantic names */ export async function queryTelemetry(env: Env, query: string): Promise { if (!env.CF_ACCOUNT_ID || !env.CF_API_TOKEN) { @@ -357,7 +653,19 @@ export async function queryTelemetry(env: Env, query: string): Promise }; } - const validationError = validateTelemetryQuery(query); + // Load schema map (canon-first, baseline fallback) + const schemaMap = await getSchemaMap(env); + + // Reject raw slot names before any other validation + const rawSlotError = detectRawSlotNames(query, schemaMap); + if (rawSlotError) { + return { error: rawSlotError }; + } + + // Rewrite semantic names → raw slot names for the CF API + const rawQuery = rewriteSqlToRaw(query, schemaMap); + + const validationError = validateTelemetryQuery(rawQuery); if (validationError) { return { error: validationError }; } @@ -370,9 +678,12 @@ export async function queryTelemetry(env: Env, query: string): Promise Authorization: `Bearer ${env.CF_API_TOKEN}`, "Content-Type": "text/plain", }, - body: query, + body: rawQuery, }, ); - return response.json(); + const result = await response.json(); + + // Rewrite result column names from raw slots back to semantic names + return rewriteResultToSemantic(result, schemaMap); } diff --git a/workers/test/telemetry-integration.test.mjs b/workers/test/telemetry-integration.test.mjs index 03a9155..b79e50c 100644 --- a/workers/test/telemetry-integration.test.mjs +++ b/workers/test/telemetry-integration.test.mjs @@ -49,6 +49,7 @@ const tsconfig = { include: [ join(WORKERS_ROOT, "src", "tokenize.ts"), join(WORKERS_ROOT, "src", "telemetry.ts"), + join(WORKERS_ROOT, "src", "zip-baseline-fetcher.ts"), ], }; const tsconfigPath = join(tmp, "tsconfig.json"); @@ -74,7 +75,8 @@ const compile = spawnSync("npx", ["--yes", "tsc", "-p", tsconfigPath], { // actually need weren't emitted. const tokenizeJs = join(tmp, "build", "tokenize.js"); const telemetryJs = join(tmp, "build", "telemetry.js"); -if (!existsSync(tokenizeJs) || !existsSync(telemetryJs)) { +const zipFetcherJs = join(tmp, "build", "zip-baseline-fetcher.js"); +if (!existsSync(tokenizeJs) || !existsSync(telemetryJs) || !existsSync(zipFetcherJs)) { console.error("TypeScript compile failed (target files not emitted):"); console.error(compile.stdout); console.error(compile.stderr); @@ -86,14 +88,25 @@ if (compile.status !== 0 && process.env.DEBUG) { } // Newer Node requires `with { type: "json" }` on JSON imports in ESM. -// TypeScript doesn't add this — patch it in. -const { readFileSync, writeFileSync: wf } = await import("node:fs"); -let telemetrySrc = readFileSync(telemetryJs, "utf8"); -telemetrySrc = telemetrySrc.replace( - /from ["']\.\.\/package\.json["'];/g, - 'from "../package.json" with { type: "json" };', -); -wf(telemetryJs, telemetrySrc); +// TypeScript bundler moduleResolution omits .js extensions on local imports. +// Node.js ESM resolver requires explicit extensions — patch all compiled files. +const { readFileSync, writeFileSync: wf, readdirSync: rds } = await import("node:fs"); +const buildDir = join(tmp, "build"); +for (const f of rds(buildDir).filter(n => n.endsWith(".js"))) { + const fpath = join(buildDir, f); + let src = readFileSync(fpath, "utf8"); + // Patch JSON imports + src = src.replace( + /from ["']\.\.\/package\.json["'];/g, + 'from "../package.json" with { type: "json" };', + ); + // Patch extensionless local imports (TypeScript bundler mode omits .js) + src = src.replace( + /from ["'](\.\/[^"'.]+)["'];/g, + 'from "$1.js";', + ); + wf(fpath, src); +} const { measurePayloadShape } = await import(tokenizeJs); const { recordTelemetry } = await import(telemetryJs); @@ -276,6 +289,230 @@ await test("batch JSON-RPC produces one data point per message", async () => { } }); +// ─── Semantic schema rewriting tests ────────────────────────────────────── + +const { + buildSchemaMapFromArrays, + detectRawSlotNames, + rewriteSqlToRaw, + rewriteResultToSemantic, +} = await import(telemetryJs); + +// Build a test schema map (mirrors the production baseline) +const TEST_BLOB_NAMES = [ + "event_type", "method", "tool_name", "consumer_label", "consumer_source", + "knowledge_base_url", "document_uri", "worker_version", "cache_tier", +]; +const TEST_DOUBLE_NAMES = [ + "count", "duration_ms", "bytes_in", "bytes_out", "tokens_in", "tokens_out", +]; +const testMap = buildSchemaMapFromArrays(TEST_BLOB_NAMES, TEST_DOUBLE_NAMES); + +await test("detectRawSlotNames: returns null for clean semantic query", async () => { + const result = detectRawSlotNames( + "SELECT tool_name, SUM(_sample_interval) FROM oddkit_telemetry GROUP BY tool_name", + testMap, + ); + assert.equal(result, null, "clean query should return null"); +}); + +await test("detectRawSlotNames: rejects blob1 with helpful message", async () => { + const result = detectRawSlotNames( + "SELECT blob1, blob3 FROM oddkit_telemetry", + testMap, + ); + assert.ok(result !== null, "should return error string"); + assert.ok(result.includes("blob1"), "error should mention the raw name"); + assert.ok(result.includes("event_type"), "error should suggest semantic name"); + assert.ok(result.includes("tool_name"), "error should suggest tool_name for blob3"); +}); + +await test("detectRawSlotNames: rejects double5 with helpful message", async () => { + const result = detectRawSlotNames( + "SELECT SUM(double5) AS x FROM oddkit_telemetry", + testMap, + ); + assert.ok(result !== null, "should return error string"); + assert.ok(result.includes("double5"), "error should mention the raw name"); + assert.ok(result.includes("tokens_in"), "error should suggest semantic name"); +}); + +await test("rewriteSqlToRaw: translates all blob semantic names", async () => { + const sql = "SELECT event_type, method, tool_name, consumer_label, consumer_source, knowledge_base_url, document_uri, worker_version, cache_tier FROM oddkit_telemetry"; + const rewritten = rewriteSqlToRaw(sql, testMap); + assert.ok(rewritten.includes("blob1"), "event_type → blob1"); + assert.ok(rewritten.includes("blob2"), "method → blob2"); + assert.ok(rewritten.includes("blob3"), "tool_name → blob3"); + assert.ok(rewritten.includes("blob6"), "knowledge_base_url → blob6"); + assert.ok(rewritten.includes("blob9"), "cache_tier → blob9"); + assert.ok(!rewritten.includes("event_type"), "event_type should be gone"); +}); + +await test("rewriteSqlToRaw: translates all double semantic names", async () => { + const sql = "SELECT SUM(count) AS n, AVG(duration_ms), SUM(bytes_in), SUM(bytes_out), AVG(tokens_in), AVG(tokens_out) FROM oddkit_telemetry"; + const rewritten = rewriteSqlToRaw(sql, testMap); + assert.ok(rewritten.includes("double1"), "count → double1"); + assert.ok(rewritten.includes("double2"), "duration_ms → double2"); + assert.ok(rewritten.includes("double3"), "bytes_in → double3"); + assert.ok(rewritten.includes("double4"), "bytes_out → double4"); + assert.ok(rewritten.includes("double5"), "tokens_in → double5"); + assert.ok(rewritten.includes("double6"), "tokens_out → double6"); + assert.ok(!rewritten.includes("duration_ms"), "duration_ms should be gone"); + assert.ok(!rewritten.includes("tokens_out"), "tokens_out should be gone"); +}); + +await test("rewriteSqlToRaw: knowledge_base_url doesn't clobber shorter substrings", async () => { + // 'url' as alias should not be mistaken for a semantic column name + // and 'knowledge_base_url' should replace as a whole unit + const sql = "SELECT knowledge_base_url AS url FROM oddkit_telemetry"; + const rewritten = rewriteSqlToRaw(sql, testMap); + assert.ok(rewritten.includes("blob6"), "knowledge_base_url → blob6"); + assert.ok(rewritten.includes("AS url"), "alias 'url' should be untouched"); +}); + +await test("rewriteSqlToRaw: count() SQL aggregate is not rewritten to double1()", async () => { + // `count` is both a semantic column name (double1) and a SQL aggregate + // function. Rewriting `count(*)` to `double1(*)` would produce invalid SQL + // that CF rejects. A function-call guard (negative lookahead for `(`) keeps + // the aggregate intact while still rewriting column references to `count`. + const sql = "SELECT tool_name, count(*) AS n FROM oddkit_telemetry GROUP BY tool_name"; + const rewritten = rewriteSqlToRaw(sql, testMap); + assert.ok(rewritten.includes("count(*)"), "count(*) aggregate should be preserved"); + assert.ok(!rewritten.includes("double1(*)"), "count(*) must not become double1(*)"); + assert.ok(rewritten.includes("blob3"), "tool_name should still rewrite to blob3"); + + // Lowercase count( with whitespace also preserved + const sql2 = "SELECT count (DISTINCT tool_name) FROM oddkit_telemetry"; + const rewritten2 = rewriteSqlToRaw(sql2, testMap); + assert.ok(!rewritten2.includes("double1 ("), "count (DISTINCT ...) must not be rewritten"); + + // But a bare `count` column reference (no paren) still rewrites + const sql3 = "SELECT SUM(count) AS n FROM oddkit_telemetry"; + const rewritten3 = rewriteSqlToRaw(sql3, testMap); + assert.ok(rewritten3.includes("SUM(double1)"), "count as column reference should still rewrite to double1"); +}); + +await test("rewriteSqlToRaw: semantic names inside single-quoted literals are preserved", async () => { + // Word-boundary regex without literal-skipping would corrupt filter values + // because `-`, `/`, and `'` are non-word characters that form `\b` boundaries. + // A query like WHERE document_uri = 'klappy://sources/scientific-method' would + // have `method` rewritten to `blob2` *inside the literal*, silently breaking + // the filter. The fix splits SQL into non-literal and single-quoted segments + // and only rewrites non-literal portions. + const sql = "SELECT tool_name FROM oddkit_telemetry WHERE document_uri = 'klappy://sources/scientific-method'"; + const rewritten = rewriteSqlToRaw(sql, testMap); + assert.ok( + rewritten.includes("'klappy://sources/scientific-method'"), + "literal value with `method` substring must be preserved verbatim", + ); + assert.ok( + !rewritten.includes("scientific-blob2"), + "method must not be rewritten inside the literal", + ); + // Column references outside the literal still rewrite correctly + assert.ok(rewritten.includes("blob3"), "tool_name column reference still rewrites to blob3"); + assert.ok(rewritten.includes("blob7"), "document_uri column reference still rewrites to blob7"); + + // SQL doubled-quote escape ('') must not terminate the literal prematurely. + // The literal here is: it''s a method — single string with one apostrophe. + // Naive splitting would treat the first '' as end-of-literal-then-start, exposing + // ` a method ` to rewriting and producing ` a blob2 `. + const sql2 = "SELECT tool_name FROM oddkit_telemetry WHERE document_uri = 'it''s a method'"; + const rewritten2 = rewriteSqlToRaw(sql2, testMap); + assert.ok( + rewritten2.includes("'it''s a method'"), + "doubled-quote escape must keep `method` inside the literal preserved", + ); + assert.ok( + !rewritten2.includes("a blob2"), + "method inside escaped-quote literal must not become blob2", + ); + + // A semantic name BOTH inside and outside a literal: only the outside one rewrites + const sql3 = "SELECT method FROM oddkit_telemetry WHERE document_uri = 'log/method/handler'"; + const rewritten3 = rewriteSqlToRaw(sql3, testMap); + assert.ok(rewritten3.startsWith("SELECT blob2"), "method as column ref rewrites to blob2"); + assert.ok( + rewritten3.includes("'log/method/handler'"), + "method inside literal stays as method", + ); +}); + +await test("detectRawSlotNames: raw slot names inside literals do not trigger rejection", async () => { + // detectRawSlotNames previously matched RAW_SLOT_PATTERN against the entire + // SQL string including literals, while rewriteSqlToRaw skipped literals. The + // inconsistency caused valid semantic queries with raw-slot-shaped substrings + // in user-supplied filter values to be falsely rejected. The fix strips + // literals before scanning, matching the rewrite scoping. + const sql = "SELECT tool_name FROM oddkit_telemetry WHERE knowledge_base_url = 'https://example.com/blob1/readme'"; + const result = detectRawSlotNames(sql, testMap); + assert.equal( + result, + null, + "blob1 inside a literal must not trigger rejection", + ); + + // Same for double-shaped slot names inside literals + const sql2 = "SELECT tool_name FROM oddkit_telemetry WHERE document_uri = 'klappy://reports/double5-summary'"; + const result2 = detectRawSlotNames(sql2, testMap); + assert.equal( + result2, + null, + "double5 inside a literal must not trigger rejection", + ); + + // Sanity check: raw slot OUTSIDE a literal must STILL be rejected. + // This guards against a future refactor that over-strips and lets real + // raw-slot references through. + const sql3 = "SELECT blob1 FROM oddkit_telemetry WHERE knowledge_base_url = 'https://example.com/safe/path'"; + const result3 = detectRawSlotNames(sql3, testMap); + assert.ok( + result3 !== null, + "bare blob1 outside any literal must still be rejected", + ); + assert.ok( + result3.includes("blob1"), + "rejection message names the offending raw slot", + ); + + // Mixed case: raw slot in a literal AND outside — must be rejected for the outside one + const sql4 = "SELECT double5 FROM oddkit_telemetry WHERE document_uri = 'safe-blob1-string'"; + const result4 = detectRawSlotNames(sql4, testMap); + assert.ok(result4 !== null, "raw slot outside literal triggers rejection even when another raw slot is in a literal"); + assert.ok(result4.includes("double5"), "rejection message names the outside-literal raw slot"); + assert.ok(!result4.includes("blob1"), "raw slot only inside literal must not be flagged"); +}); + +await test("rewriteResultToSemantic: renames blob/double columns in meta and data", async () => { + const rawResult = { + meta: [ + { name: "blob3", type: "String" }, + { name: "double2", type: "Float64" }, + { name: "total", type: "UInt64" }, + ], + data: [ + { blob3: "search", double2: 123.4, total: "42" }, + { blob3: "orient", double2: 88.0, total: "17" }, + ], + rows: 2, + }; + const result = rewriteResultToSemantic(rawResult, testMap); + assert.deepEqual(result.meta[0], { name: "tool_name", type: "String" }, "blob3 → tool_name in meta"); + assert.deepEqual(result.meta[1], { name: "duration_ms", type: "Float64" }, "double2 → duration_ms in meta"); + assert.deepEqual(result.meta[2], { name: "total", type: "UInt64" }, "non-slot column unchanged"); + assert.equal(result.data[0].tool_name, "search", "data row key renamed"); + assert.equal(result.data[0].duration_ms, 123.4, "double2 key renamed"); + assert.equal(result.data[0].total, "42", "non-slot key unchanged"); + assert.ok(!("blob3" in result.data[0]), "old key blob3 removed"); + assert.ok(!("double2" in result.data[0]), "old key double2 removed"); +}); + +await test("rewriteResultToSemantic: passes through non-slot result unchanged", async () => { + const rawResult = { error: "bad query" }; + const result = rewriteResultToSemantic(rawResult, testMap); + assert.deepEqual(result, rawResult, "error result passed through unchanged"); +}); + // ─── Test 5: Malformed JSON-RPC gets dropped silently ────────────────────── await test("malformed JSON-RPC is silently dropped (telemetry never throws)", async () => {