test: gate the frames-per-request ceiling across server and client

.github/path-filters.yml

@@ -67,6 +67,10 @@ frontend:
   # to `python`, whose pytest never collects them. Without this line, editing one
   # of those tests would run every suite except the one being edited.
   - "tests/scripts/**"
+  # Read as text by `tests/scripts/frames_per_request.test.mjs`, which holds the
+  # browser's upload chunk to this route's bound; a change to the bound alone has
+  # to wake the job that runs it.
+  - "src/visionset/server/routes/video_imports.py"
   - ".github/workflows/ci.yml"
   - ".github/path-filters.yml"
 

frontend/app/cycle/video-import.spec.ts

@@ -239,6 +239,10 @@ async function watch(page: Page): Promise<Log> {
         // silently counted as a body with nothing in it.
         unreadable = true;
       }
+      const signal = init?.signal ?? (input instanceof Request ? input.signal : null);
+      // Aborted while the clone was being read: fetch rejects without a request ever
+      // reaching Chromium, so it did not travel and the wire count cannot include it.
+      if (signal?.aborted) return native(input, init);
       sent.push({
         method: probe.method,
         path: new URL(probe.url, location.href).pathname,

frontend/app/src/data/frameSink.test.ts

@@ -19,15 +19,12 @@ import { describe, expect, it } from "vitest";
 
 import type { MaterializedFrame } from "@visionset/media";
 
-import { createLocalApiFrameSink } from "./frameSink";
+import { createLocalApiFrameSink, FRAMES_PER_REQUEST } from "./frameSink";
 import { createOssDataClient } from "./ossClient";
 
 const BASE = "http://visionset.test";
 const TARGET = { projectId: "p", importId: "i" };
 
-/** The bound the sink slices at, restated so a change to it fails here loudly. */
-const FRAMES_PER_REQUEST = 32;
-
 const IMPORT = {
   id: "i",
   project_id: "p",
@@ -117,7 +114,9 @@ describe("what one append puts on the wire", () => {
     // did not slice with its files would attach every frame's metadata to the wrong
     // frame, and every part would still be well-formed.
     expect(first.descriptors).toHaveLength(FRAMES_PER_REQUEST);
-    expect(second.descriptors.map((one) => one.ordinal)).toEqual([32, 33, 34, 35, 36]);
+    expect(second.descriptors.map((one) => one.ordinal)).toEqual(
+      Array.from({ length: 5 }, (_, at) => FRAMES_PER_REQUEST + at),
+    );
   });
 
   it("sends nothing at all for an empty chunk", async () => {

frontend/app/src/data/frameSink.ts

@@ -20,11 +20,12 @@ import type { AbortSignal as AbortLike, FrameSink, MaterializedFrame } from "@vi
 /**
  * How many frame parts one request may carry — the server's own bound
  * (`routes/video_imports.py`), restated here because exceeding it is a 422 rather
- * than something a type could catch. The materializer's `FRAME_CHUNK_SIZE` is
- * smaller, so the loop below normally runs once; it exists because the sink's
- * contract takes an array of any length, not because the current caller needs it.
+ * than something a type could catch. `tests/scripts/frames_per_request.test.mjs`
+ * holds the two equal. The materializer's `FRAME_CHUNK_SIZE` is smaller, so the
+ * loop below normally runs once; it exists because the sink's contract takes an
+ * array of any length, not because the current caller needs it.
  */
-const FRAMES_PER_REQUEST = 32;
+export const FRAMES_PER_REQUEST = 32;
 
 /** The parallel array the frames route reads, one entry per uploaded part. */
 function descriptorsOf(frames: readonly MaterializedFrame[]): string {

tests/scripts/ci_path_filters.test.mjs

@@ -89,6 +89,10 @@ test("these script tests wake the job that actually runs them", () => {
   );
 });
 
+test("a server file a script gate reads wakes the job that runs the gate", () => {
+  assert.deepEqual(groupsFor("src/visionset/server/routes/video_imports.py"), ["docker", "frontend", "python"]);
+});
+
 test("the API contract wakes both sides of it", () => {
   // Python regenerates it and fails on drift; the frontend regenerates its client
   // from it and fails on drift. A change here has to face both.

tests/scripts/frames_per_request.test.mjs

@@ -0,0 +1,76 @@
+// Run with: pnpm test:scripts  (also part of the root `pnpm test`)
+//
+// The frames route refuses a request carrying more than `FRAMES_PER_REQUEST` parts
+// with a 422, and the browser's frame sink slices its uploads at its own copy of
+// that number. The generated client is types only, so even a `maxItems` in the
+// schema would reach the sink as no value at all — nothing but this gate holds the
+// copies together (#849). The route's docstring states it too, and that prose
+// is published in `openapi.json` as the operation's description.
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import { test } from "node:test";
+
+const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
+
+const ROUTE = "src/visionset/server/routes/video_imports.py";
+const SINK = "frontend/app/src/data/frameSink.ts";
+const SINK_TEST = "frontend/app/src/data/frameSink.test.ts";
+
+const SPELLINGS = [
+  { file: ROUTE, label: "the enforced constant", pattern: /^FRAMES_PER_REQUEST: Final = (\d+)$/gm },
+  { file: ROUTE, label: "the route's docstring", pattern: /at most (\d+) parts per request/g },
+  { file: SINK, label: "the client's chunk size", pattern: /^export const FRAMES_PER_REQUEST = (\d+);$/gm },
+];
+
+const read = (file) => readFileSync(path.join(ROOT, file), "utf8");
+
+/** Every spelling's value, or a reason it could not be read as exactly one. */
+function measure(texts) {
+  return SPELLINGS.map(({ file, label, pattern }) => {
+    const found = [...texts[file].matchAll(pattern)].map((match) => Number(match[1]));
+    return { file, label, found };
+  });
+}
+
+function disagreements(texts) {
+  const spellings = measure(texts);
+  const problems = spellings
+    .filter(({ found }) => found.length !== 1)
+    .map(({ file, label, found }) => `${file}: expected exactly one match for ${label}, found ${found.length}`);
+  if (problems.length > 0) return problems;
+  const values = new Set(spellings.map(({ found }) => found[0]));
+  if (values.size === 1) return [];
+  return [
+    "FRAMES_PER_REQUEST disagrees across server and client — the server's value is the bound, " +
+      "the others must follow it:\n" +
+      spellings.map(({ file, label, found }) => `  ${file} (${label}): ${found[0]}`).join("\n"),
+  ];
+}
+
+const committed = () => ({ [ROUTE]: read(ROUTE), [SINK]: read(SINK) });
+
+test("the server's bound and the client's chunk size are one number", () => {
+  assert.deepEqual(disagreements(committed()), []);
+});
+
+test("the sink's test takes the bound from the sink instead of restating it", () => {
+  const text = read(SINK_TEST);
+  assert.match(text, /import \{[^}]*\bFRAMES_PER_REQUEST\b[^}]*\} from "\.\/frameSink";/);
+  assert.doesNotMatch(text, /\bconst FRAMES_PER_REQUEST\b/, `${SINK_TEST} declares its own copy`);
+});
+
+test("the gate fires on a drifted copy", () => {
+  const texts = committed();
+  const drifted = { ...texts, [SINK]: texts[SINK].replace(/(export const FRAMES_PER_REQUEST = )\d+;/, "$164;") };
+  const [problem] = disagreements(drifted);
+  assert.ok(problem, "a client copy of 64 against the server's bound passed");
+  for (const file of [ROUTE, SINK]) assert.ok(problem.includes(file), `the message does not name ${file}`);
+});
+
+test("the gate fires when a spelling disappears", () => {
+  const texts = committed();
+  const renamed = { ...texts, [ROUTE]: texts[ROUTE].replace("FRAMES_PER_REQUEST: Final", "MAX_FRAMES: Final") };
+  assert.deepEqual(disagreements(renamed), [`${ROUTE}: expected exactly one match for the enforced constant, found 0`]);
+});