Bläddra i källkod

fix(code-runtime-python): count non-lossless bytes and bind the mirror gate to TS types

Two gaps from the previous round's fixes:

- checkDoneValue flagged a non-lossless number but skipped counting its encoded
  bytes, so a value over budget ONLY through that number classified as
  non-lossless instead of over-budget (e.g. [Infinity] at cap 3, whose encoding
  is 10 bytes). Count the scalar's bytes even when flagging, so the budget check
  wins as the JSDoc promises. Add cap-3 regression cases.

- The mirror e2e compared the Python TypedDict keys against a hand-written
  constant, so a field change on the TS side alone would not fail it, and the
  reply frames were not probed at all. Introduce WIRE_FRAME_FIELDS in
  protocol.ts, bound to each frame interface's key set via `satisfies` (a
  renamed/removed field breaks typecheck — verified), and drive the mirror test
  from it, now covering ReplyOk/ReplyErr too. The test therefore fails on
  one-sided drift from either language.
Chinesezjc 2 månader sedan
förälder
incheckning
be839a8e53

+ 64 - 1
packages/code-runtime/code-runtime-python/src/protocol.ts

@@ -107,6 +107,64 @@ export type ReplyMessage =
   | { type: 'reply'; id: number; ok: true; value: unknown }
   | { type: 'reply'; id: number; ok: false; message: string }
 
+/**
+ * Shape of one {@link WIRE_FRAME_FIELDS} entry, parameterised by that frame's
+ * key union `K`. `required` and `optional` are arrays of `K`, so listing a name
+ * no frame declares — a typo or a renamed field — fails typecheck. (A field
+ * ADDED to an interface but omitted here is caught at runtime instead: the
+ * mirror test asserts the Python `TypedDict` keys equal these exact sets, and
+ * the Python side would carry the new field.) `K` is `PropertyKey` so a bare
+ * `keyof Interface` binds without narrowing.
+ */
+type FrameFields<K extends PropertyKey> = {
+  required: readonly K[]
+  optional: readonly K[]
+}
+
+/**
+ * The wire field names of each frame, split into required and optional keys, as
+ * a RUNTIME value the cross-language mirror test asserts `py/protocol.py`'s
+ * `TypedDict`s against. The `satisfies` clause binds each entry to its frame
+ * interface's own key set, so listing a name no frame declares fails
+ * typecheck — the mirror test therefore depends on the TS declarations above,
+ * not a hand-copied list. `global` is the JSON key {@link CallMessage} and the
+ * namespace declaration send (a reserved word the Python side carries via a
+ * functional `TypedDict`); inline sub-shapes (the namespace entry in
+ * {@link BootMessage}, the error field in {@link DoneMessage}, the reply
+ * variants) list their keys literally.
+ */
+export const WIRE_FRAME_FIELDS = {
+  BootMessage: { required: ['addressSpaceBytes', 'cpuSeconds', 'maxLogBytes', 'maxValueBytes', 'namespaces', 'type'], optional: [] },
+  Namespace: { required: ['global', 'names'], optional: ['errorClass'] },
+  RunMessage: { required: ['program', 'type'], optional: [] },
+  BootAckMessage: { required: ['type'], optional: [] },
+  CallMessage: { required: ['args', 'global', 'id', 'name', 'type'], optional: [] },
+  LogMessage: { required: ['text', 'type'], optional: ['truncated'] },
+  DoneErrorField: { required: ['kind', 'message'], optional: [] },
+  DoneMessage: { required: ['type'], optional: ['error', 'value'] },
+  ErrorClass: { required: ['name', 'memberNameProperty'], optional: [] },
+  ReplyOk: { required: ['id', 'ok', 'type', 'value'], optional: [] },
+  ReplyErr: { required: ['id', 'message', 'ok', 'type'], optional: [] },
+} satisfies {
+  // Frames with a top-level interface bind to its keys; `global` is already the
+  // member name on the TS side of `CallMessage`. Frames sent as inline literals
+  // or nested shapes (the run frame, the namespace entry, the done error field,
+  // ErrorClass, and the two reply variants) have no standalone interface, so
+  // their keys are listed literally.
+  BootMessage: FrameFields<keyof BootMessage>
+  Namespace: FrameFields<'global' | 'names' | 'errorClass'>
+  RunMessage: FrameFields<'type' | 'program'>
+  BootAckMessage: FrameFields<keyof BootAckMessage>
+  CallMessage: FrameFields<keyof CallMessage>
+  LogMessage: FrameFields<keyof LogMessage>
+  DoneErrorField: FrameFields<'kind' | 'message'>
+  DoneMessage: FrameFields<keyof DoneMessage>
+  ErrorClass: FrameFields<'name' | 'memberNameProperty'>
+  ReplyOk: FrameFields<'type' | 'id' | 'ok' | 'value'>
+  ReplyErr: FrameFields<'type' | 'id' | 'ok' | 'message'>
+}
+
+
 /**
  * The in-band marker text announcing that log capture stopped at the byte
  * budget. Shared wire vocabulary: the Python-side LogBuffer emits it when ITS
@@ -230,8 +288,13 @@ export function checkDoneValue(value: unknown, maxBytes: number): { ok: true; by
   while (stack.length > 0) {
     const current = stack.pop()
     if (typeof current === 'number') {
+      // Flag a non-lossless number but keep counting its encoded bytes: a value
+      // that is BOTH non-lossless and over-budget must classify as over-budget
+      // (the loop's byte check below wins), so the byte count cannot skip the
+      // offending number. `scalarJson` gives the same spelling a legit scalar
+      // would meter.
       if (!Number.isFinite(current) || Object.is(current, -0)) nonLossless = true
-      else bytes += Buffer.byteLength(scalarJson(current), 'utf8')
+      bytes += Buffer.byteLength(scalarJson(current), 'utf8')
     } else if (typeof current === 'string') {
       // Lower-bound BEFORE materializing the escaped form: every UTF-16 code
       // unit is at least one UTF-8 byte plus the two quotes, so a huge or

+ 19 - 30
packages/code-runtime/code-runtime-python/tests/protocol-mirror.e2e.ts

@@ -3,7 +3,7 @@ import { existsSync } from 'node:fs'
 import { fileURLToPath } from 'node:url'
 import { promisify } from 'node:util'
 import { describe, expect, it } from 'vitest'
-import { logTruncationMarker } from '../src/protocol.ts'
+import { logTruncationMarker, WIRE_FRAME_FIELDS } from '../src/protocol.ts'
 
 /**
  * Cross-language mirror check between `src/protocol.ts` and `py/protocol.py`,
@@ -56,43 +56,32 @@ describe.skipIf(!python3Available)('protocol.py mirrors protocol.ts at runtime',
   it('agrees on every frame type\'s wire field set between the TS and Python declarations', async () => {
     // Turn the TypedDict mirror from a review-only obligation into an executable
     // check: read each Python TypedDict's required/optional key sets and assert
-    // them against the wire field names the TS side declares. `global` is the
-    // reserved-keyword key the Python side carries via functional TypedDict —
-    // catching exactly the round-12 kind of drift (a renamed/dropped field, an
-    // optional field the other side made required).
+    // them against WIRE_FRAME_FIELDS — the TS-side source of truth bound to the
+    // frame interfaces by `satisfies` in protocol.ts, so a rename or a removed
+    // field on the TS side breaks typecheck and an added field breaks this
+    // comparison (the Python side would carry it). Covers the reply frames too.
+    // `global` is the reserved-keyword wire key the Python side carries via a
+    // functional TypedDict. This catches the round-12 kind of drift on EITHER
+    // side of the wire.
+    const pyNames = Object.keys(WIRE_FRAME_FIELDS)
     const probe = [
       'import json, sys',
       `sys.path.insert(0, ${JSON.stringify(pyDir)})`,
       'import protocol as p',
       'def keys(td): return {"required": sorted(td.__required_keys__), "optional": sorted(td.__optional_keys__)}',
-      'print(json.dumps({',
-      '  "BootMessage": keys(p.BootMessage),',
-      '  "Namespace": keys(p.Namespace),',
-      '  "RunMessage": keys(p.RunMessage),',
-      '  "BootAckMessage": keys(p.BootAckMessage),',
-      '  "CallMessage": keys(p.CallMessage),',
-      '  "LogMessage": keys(p.LogMessage),',
-      '  "DoneErrorField": keys(p.DoneErrorField),',
-      '  "DoneMessage": keys(p.DoneMessage),',
-      '  "ErrorClass": keys(p.ErrorClass),',
-      '}))',
+      `names = ${JSON.stringify(pyNames)}`,
+      'print(json.dumps({n: keys(getattr(p, n)) for n in names}))',
     ].join('\n')
     const { stdout } = await execFileAsync('python3', ['-I', '-c', probe])
     const seen = JSON.parse(stdout) as Record<string, { required: string[]; optional: string[] }>
-    // The wire field sets each frame carries, mirroring src/protocol.ts. `global`
-    // is the JSON key `CallMessage`/`Namespace` send (a Python keyword, declared
-    // functionally on the Python side).
-    expect(seen).toEqual({
-      BootMessage: { required: ['addressSpaceBytes', 'cpuSeconds', 'maxLogBytes', 'maxValueBytes', 'namespaces', 'type'], optional: [] },
-      Namespace: { required: ['global', 'names'], optional: ['errorClass'] },
-      RunMessage: { required: ['program', 'type'], optional: [] },
-      BootAckMessage: { required: ['type'], optional: [] },
-      CallMessage: { required: ['args', 'global', 'id', 'name', 'type'], optional: [] },
-      LogMessage: { required: ['text', 'type'], optional: ['truncated'] },
-      DoneErrorField: { required: ['kind', 'message'], optional: [] },
-      DoneMessage: { required: ['type'], optional: ['error', 'value'] },
-      ErrorClass: { required: ['memberNameProperty', 'name'], optional: [] },
-    })
+    // Normalize the TS source of truth to the same sorted shape Python reports.
+    const expected = Object.fromEntries(
+      Object.entries(WIRE_FRAME_FIELDS).map(([name, sets]) => [
+        name,
+        { required: [...sets.required].sort(), optional: [...sets.optional].sort() },
+      ]),
+    )
+    expect(seen).toEqual(expected)
   })
 })
 

+ 6 - 0
packages/code-runtime/code-runtime-python/tests/protocol.spec.ts

@@ -262,6 +262,12 @@ describe('checkDoneValue', () => {
     // non-lossless (the recorded violation is the verdict once the whole value
     // is confirmed within budget).
     expect(checkDoneValue([Infinity], 100)).toEqual({ ok: false, reason: 'non-lossless' })
+    // The non-lossless number's OWN encoded bytes still count toward the budget,
+    // so a value whose only over-budget contribution is the non-lossless number
+    // itself is classified over-budget, not non-lossless. `[Infinity]` encodes
+    // as the 10-byte `[Infinity]`; at cap 3 the byte check wins.
+    expect(checkDoneValue([Infinity], 3)).toEqual({ ok: false, reason: 'over-budget' })
+    expect(checkDoneValue(Infinity, 3)).toEqual({ ok: false, reason: 'over-budget' })
   })
 
   it('meters deep nesting iteratively without overflowing the stack', () => {