Merge pull request #134 from klappy/feat/telemetry-tokenization

[klappy]

Note

Medium Risk
Adds new telemetry fields and rewrites the telemetry query path (semantic-to-raw SQL translation plus result rewriting), which could affect observability accuracy and query behavior if parsing/rewrite edge cases slip through.

Overview
Adds payload-shape telemetry to the Worker by measuring request/response bytes_* and tokens_* (via lazy-loaded gpt-tokenizer cl100k) and writing them as double3–double6 in oddkit_telemetry, with response cloning + measurement deferred in ctx.waitUntil (and no Content-Type filtering).

Refactors telemetry recording to accept the already-read request body and payload shape, fixes a streaming race by reading tracer.indexSource only after response body consumption, and expands cache-tier detection to treat file:* spans as primary fetches.

Upgrades telemetry_public querying to a semantic schema contract: loads a canon-derived mapping (baseline fallback), rejects raw blobN/doubleN references, rewrites consumer SQL to raw slots for Cloudflare, and rewrites result column names back to semantic. Adds new unit/integration tests for tokenization, telemetry write path, and SQL rewriting, and bumps root/worker versions to 0.24.0 (plus adds gpt-tokenizer dependency).

^{Reviewed by Cursor Bugbot for commit e2912f3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	oddkit	e2912f3	Commit Preview URL Branch Preview URL	Apr 26 2026, 03:04 AM

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Unprotected response.clone() can crash MCP handler
  • Wrapped the synchronous response.clone() call in a try/catch so a clone failure falls back to skipping telemetry and lets return response execute, upholding the invariant that telemetry must never break MCP requests.

Preview (be41e0c37f)

diff --git a/workers/src/index.ts b/workers/src/index.ts
--- a/workers/src/index.ts
+++ b/workers/src/index.ts
@@ -970,29 +970,37 @@
         const cacheTier = tracer.indexSource;
         // Clone the response synchronously before returning so the body is
         // still available to read inside the deferred waitUntil callback.
-        const responseClone = response.clone();
+        // Guard the clone itself — telemetry must never break MCP requests.
+        let responseClone: Response | null = null;
+        try {
+          responseClone = response.clone();
+        } catch {
+          // If cloning fails, skip telemetry for this request.
+        }
 
-        ctx.waitUntil(
-          (async () => {
-            try {
-              const requestText = await telemetryClone.text();
+        if (responseClone) {
+          ctx.waitUntil(
+            (async () => {
+              try {
+                const requestText = await telemetryClone.text();
 
-              const { measurePayloadShape } = await import("./tokenize");
-              const { recordTelemetry } = await import("./telemetry");
+                const { measurePayloadShape } = await import("./tokenize");
+                const { recordTelemetry } = await import("./telemetry");
 
-              let responseText = "";
-              try {
-                responseText = await responseClone.text();
+                let responseText = "";
+                try {
+                  responseText = await responseClone!.text();
+                } catch {
+                  // Fall through with empty string; bytes_out / tokens_out will be 0.
+                }
+                const shape = await measurePayloadShape(requestText, responseText);
+                recordTelemetry(request, requestText, env, durationMs, cacheTier, shape);
               } catch {
-                // Fall through with empty string; bytes_out / tokens_out will be 0.
+                // Telemetry must never break MCP requests
               }
-              const shape = await measurePayloadShape(requestText, responseText);
-              recordTelemetry(request, requestText, env, durationMs, cacheTier, shape);
-            } catch {
-              // Telemetry must never break MCP requests
-            }
-          })(),
-        );
+            })(),
+          );
+        }
       }
 
       return response;

_{You can send follow-ups to the cloud agent here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Canon-derived names used in regex without escaping
  • Added an escapeRegExp helper and used it on the semantic name when building the rewrite regex in rewriteSqlToRaw, so canon-derived names containing regex metacharacters can no longer corrupt the pattern.

Preview (838d97cd27)

diff --git a/workers/src/telemetry.ts b/workers/src/telemetry.ts
--- a/workers/src/telemetry.ts
+++ b/workers/src/telemetry.ts
@@ -456,6 +456,16 @@
 const RAW_SLOT_PATTERN = /\b(blob[1-9]|double[1-9])\b/gi;
 
 /**
+ * Escape regex metacharacters in a string so it can be safely interpolated
+ * into a `new RegExp(...)` pattern. Used for canon-derived semantic names,
+ * which are sourced from a maintainer-controlled markdown table but could
+ * in principle contain characters like `.` that have special regex meaning.
+ */
+function escapeRegExp(s: string): string {
+  return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+/**
  * Reject queries that contain raw slot names (blob1..9 / double1..6).
  * Returns an error message string, or null if the query is clean.
  *
@@ -522,7 +532,10 @@
     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");
+      const pattern = new RegExp(
+        `\\b${escapeRegExp(semantic)}\\b(?!\\s*\\()`,
+        "g",
+      );
       out = out.replace(pattern, raw);
     }
     return out;

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit 838d97c. Configure here.}

[cursor]

Canon-derived names used in regex without escaping

Low Severity

In rewriteSqlToRaw, semantic names are interpolated directly into new RegExp() without escaping regex metacharacters. The baseline names are safe (only [a-z_]), but parseDoublesFromCanon accepts backtick-quoted identifiers containing arbitrary characters (e.g., ., (, +). If canon ever provides such a name, it would corrupt the regex and produce incorrect or broken SQL rewriting.

 

^{Reviewed by Cursor Bugbot for commit 838d97c. Configure here.}