analysis.js: the blank-first ordering routes an out-of-range reply to abstain, against its own comment and the corpus's fail_closed label #73

Closed
opened 2026-10-11 12:28:07 +00:00 by rockachopa · 3 comments
Member

Filed by the control plane (Timmy) after PR #72 merged, because the ordering question below was
raised for exact-head hostile review and the head landed before any review could run (Praetorian's provider
auth was failing at 08:14Z; no other reviewer was in place). It is not a safety breach — the path prefills
nothing, returns no provider text and no image bytes — it is a contract mismatch on a P0 surface, and it is
one line to decide.

Where

src/analysis.js, at main head a1777a72e (merge of PR #72 / d9f82299d), 125 lines:

:37  // The request schema lets the model return a null form and color when it cannot read them.
:38  // A schema-valid answer with nothing to prefill abstains; an out-of-range value still fails closed.
:39  const blank = input => input === null || input === undefined || (typeof input === 'string' && !input.trim());
:40  if (blank(value.bristolType) || blank(value.color)) {
:41    return { status: 'needs_user_input', isStool: true,
:42             abstainedFor: 'unusable_visual_fields', reason: '...' };
:47  const bristolType = Number(value.bristolType);
:49  if (!Number.isInteger(bristolType) || bristolType < 1 || bristolType > 7 || !COLORS.has(color)) {
:50    throw visionResponseError(VISION_FAILURE_CODES.invalidFields, 'Invalid AI response: visual fields are out of range.');

The class that escapes the contract

A reply that is both out-of-range and blank — e.g. {"bristolType": 9, "color": null} — enters the
blank check at :40 first and returns the abstention, never reaching the range check at :49.

Two of the repo's own statements say it should fail closed:

  • the comment at :38 — "an out-of-range value still fails closed";
  • tests/fixtures/vision-corpus/manifest.json, whose out-of-range case carries
    expectedReason: out_of_range_or_missing_fields with expectedOutcome: fail_closed.

So "missing" fields are carved out of a class the corpus labels fail-closed. The corpus has no case for the
combination: its only out-of-range fixture (out-of-range-fields) carries a non-null "purple" colour, so
the shipped boundary test's fail-closed assertion for out-of-range passes while this subclass abstains.

Why it matters, bounded

Not a safety hole. But issue #18's AC 2 is a fail-closed guarantee, and this narrows it silently: a caller
reading abstainedFor: unusable_visual_fields is told "the model could not read the photo", not "the model
returned an impossible value" — which is the distinction the corpus was built to pin down.

Minimal fix, for whoever owns it

Either

  • move the blank check after the range check, so a non-blank-but-out-of-range value still throws; or
  • keep the ordering and add the combined case to the corpus with expectedOutcome set to whatever is decided,
    so the contract and the corpus agree.

Per the standing rule this is a build-owner decision, not a control-plane one.

Filed by the control plane (Timmy) after PR #72 merged, because the ordering question below was raised for exact-head hostile review and the head landed before any review could run (Praetorian's provider auth was failing at 08:14Z; no other reviewer was in place). It is **not** a safety breach — the path prefills nothing, returns no provider text and no image bytes — it is a contract mismatch on a P0 surface, and it is one line to decide. ## Where `src/analysis.js`, at `main` head `a1777a72e` (merge of PR #72 / `d9f82299d`), 125 lines: ``` :37 // The request schema lets the model return a null form and color when it cannot read them. :38 // A schema-valid answer with nothing to prefill abstains; an out-of-range value still fails closed. :39 const blank = input => input === null || input === undefined || (typeof input === 'string' && !input.trim()); :40 if (blank(value.bristolType) || blank(value.color)) { :41 return { status: 'needs_user_input', isStool: true, :42 abstainedFor: 'unusable_visual_fields', reason: '...' }; :47 const bristolType = Number(value.bristolType); :49 if (!Number.isInteger(bristolType) || bristolType < 1 || bristolType > 7 || !COLORS.has(color)) { :50 throw visionResponseError(VISION_FAILURE_CODES.invalidFields, 'Invalid AI response: visual fields are out of range.'); ``` ## The class that escapes the contract A reply that is **both** out-of-range and blank — e.g. `{"bristolType": 9, "color": null}` — enters the blank check at `:40` first and returns the abstention, never reaching the range check at `:49`. Two of the repo's own statements say it should fail closed: - the comment at `:38` — "an out-of-range value still fails closed"; - `tests/fixtures/vision-corpus/manifest.json`, whose out-of-range case carries `expectedReason: out_of_range_or_missing_fields` with `expectedOutcome: fail_closed`. So "missing" fields are carved out of a class the corpus labels fail-closed. The corpus has no case for the combination: its only out-of-range fixture (`out-of-range-fields`) carries a non-null `"purple"` colour, so the shipped boundary test's fail-closed assertion for out-of-range passes while this subclass abstains. ## Why it matters, bounded Not a safety hole. But issue #18's AC 2 is a fail-closed guarantee, and this narrows it silently: a caller reading `abstainedFor: unusable_visual_fields` is told "the model could not read the photo", not "the model returned an impossible value" — which is the distinction the corpus was built to pin down. ## Minimal fix, for whoever owns it Either - move the blank check after the range check, so a non-blank-but-out-of-range value still throws; or - keep the ordering and add the combined case to the corpus with `expectedOutcome` set to whatever is decided, so the contract and the corpus agree. Per the standing rule this is a build-owner decision, not a control-plane one.
rockachopa added the priority/P1area/model labels 2026-10-11 12:28:07 +00:00
Author
Member

Corrections against this issue, filed by the control plane after the monitor executed the claims rather
than reading them. The body above is left as filed — the trail is the evidence — and these three items
supersede it.

1. The line numbers in the body's code block are shifted by one. Authoritative mapping, read from
src/analysis.js at a1777a72e (125 lines):

:36  // The request schema lets the model return a null form and color when it cannot read them.
:37  // A schema-valid answer with nothing to prefill abstains; an out-of-range value still fails closed.
:38  const blank = input => input === null || input === undefined || (typeof input === 'string' && !input.trim());
:39  if (blank(value.bristolType) || blank(value.color)) {
:40      return { status: 'needs_user_input', isStool: true,
:44               abstainedFor: 'unusable_visual_fields', reason: '...' };
:46  }
:47  const bristolType = Number(value.bristolType);
:48  const color = String(value.color).toLowerCase();
:49  if (!Number.isInteger(bristolType) || bristolType < 1 || bristolType > 7 || !COLORS.has(color)) {
:50      throw visionResponseError(VISION_FAILURE_CODES.invalidFields, 'Invalid AI response: visual fields are out of range.');

The body's :47 / :49 / :50 happen to be exact; the heading block's :37–:40 is what shifted.

2. The stated reason the corpus assertion passes is incomplete — the gap is worse than the body says.
The out-of-range fixture's reply is

{"isStool": true, "bristolType": 9, "color": "purple", "confidence": 1.4, "imageQuality": "excellent", ...}

and it fails closed at the confidence gate on :33 (confidence > 1 throws), not at the field-range
check. So no corpus fixture reaches :49 at all, and the range branch is covered only by a unit test,
tests/analysis.test.js:56. The body's sentence implies a fixture exercises the range check; none does.

3. The first suggested fix is wrong as written. Moving the blank check after the range check would send
the glare fixture — {"bristolType": null, "color": null, "confidence": 0.66}, which the manifest expects to
be abstained / unusable_visual_fields — into Number(null) === 0, then 0 < 1, and it would throw,
converting an expected abstention into a 503.

The fix must fail close only for non-blank out-of-range values: keep the blank-first abstention for null /
undefined / whitespace-only, and make the range clause treat blanks as already-handled rather than as 0.
Option 2 from the body stands unchanged — keep the ordering and add a corpus case for out-of-range plus a
null field, with the outcome set to whatever is decided.

The decision this still asks for, unchanged and unrecorded: is abstained / unusable_visual_fields
acceptable for a non-blank out-of-range value, where the comment on :37 and the corpus's own
out_of_range_or_missing_fields -> fail_closed label both say otherwise? The control plane's read is no, and
the fix above is the shape that matches; the call is the build owner's, not ours.

Corrections against this issue, filed by the control plane after the monitor executed the claims rather than reading them. The body above is left as filed — the trail is the evidence — and these three items supersede it. **1. The line numbers in the body's code block are shifted by one.** Authoritative mapping, read from `src/analysis.js` at `a1777a72e` (125 lines): ``` :36 // The request schema lets the model return a null form and color when it cannot read them. :37 // A schema-valid answer with nothing to prefill abstains; an out-of-range value still fails closed. :38 const blank = input => input === null || input === undefined || (typeof input === 'string' && !input.trim()); :39 if (blank(value.bristolType) || blank(value.color)) { :40 return { status: 'needs_user_input', isStool: true, :44 abstainedFor: 'unusable_visual_fields', reason: '...' }; :46 } :47 const bristolType = Number(value.bristolType); :48 const color = String(value.color).toLowerCase(); :49 if (!Number.isInteger(bristolType) || bristolType < 1 || bristolType > 7 || !COLORS.has(color)) { :50 throw visionResponseError(VISION_FAILURE_CODES.invalidFields, 'Invalid AI response: visual fields are out of range.'); ``` The body's `:47` / `:49` / `:50` happen to be exact; the heading block's `:37`–`:40` is what shifted. **2. The stated reason the corpus assertion passes is incomplete — the gap is worse than the body says.** The out-of-range fixture's reply is ``` {"isStool": true, "bristolType": 9, "color": "purple", "confidence": 1.4, "imageQuality": "excellent", ...} ``` and it fails closed at the **confidence gate on `:33`** (`confidence > 1` throws), not at the field-range check. So **no corpus fixture reaches `:49` at all**, and the range branch is covered only by a unit test, `tests/analysis.test.js:56`. The body's sentence implies a fixture exercises the range check; none does. **3. The first suggested fix is wrong as written.** Moving the blank check *after* the range check would send the `glare` fixture — `{"bristolType": null, "color": null, "confidence": 0.66}`, which the manifest expects to be `abstained` / `unusable_visual_fields` — into `Number(null) === 0`, then `0 < 1`, and it would **throw**, converting an expected abstention into a 503. The fix must fail close **only for non-blank** out-of-range values: keep the blank-first abstention for null / undefined / whitespace-only, and make the range clause treat blanks as already-handled rather than as `0`. Option 2 from the body stands unchanged — keep the ordering and add a corpus case for out-of-range **plus** a null field, with the outcome set to whatever is decided. **The decision this still asks for**, unchanged and unrecorded: is `abstained / unusable_visual_fields` acceptable for a **non-blank** out-of-range value, where the comment on `:37` and the corpus's own `out_of_range_or_missing_fields -> fail_closed` label both say otherwise? The control plane's read is no, and the fix above is the shape that matches; the call is the build owner's, not ours.
timmy was assigned by rockachopa 2026-10-11 12:53:34 +00:00
Author
Member

Second correction — two defects in the comment above, found by the monitor executing it.
(Control plane, 2026-10-11 13:15Z. The body and the first comment stay as filed; the trail is the evidence.)

a. "no corpus fixture reaches :49 at all" is false as written. Enumerating all seven cases,
stool-like-formed (Bristol 4, colour "brown", confidence 0.83) traverses :49 and passes it —
asserted at tests/vision-boundary.test.js:65-70. The accurate bound is narrower and still useful: no
corpus fixture fails at :49
, so the throw at :50 is corpus-unexercised. What dies at :33 is the
out-of-range fixture only.

b. The block above is not verbatim. The real :40-44, from a numbered read at a1777a72e:

:40      return {
:41        status: 'needs_user_input',
:42        isStool: true,
:43        abstainedFor: 'unusable_visual_fields',
:44        reason: 'The photo does not show a form Timmy can read safely.',

The anchors that decide the ordering question (:33, :36, :37, :38, :39, :46, :47, :48, :49,
:50) are exact; the return block was a compressed rendering with abstainedFor mislabelled onto :44.

c. Framing. Issue #18's AC 2 reads "Every response is schema-valid or fails closed", and the repo's own
taxonomy (src/vision-corpus.js:24) declares abstained a legitimate third outcome — so abstention does not
breach AC 2 on its face
, and the earlier text should not be read as saying it does. What is breached is the
comment at :37 and the precision the label implies. The escape class, stated exactly: at least one blank
field together with at least one out-of-range field.
A reply whose only out-of-range field is non-blank, or
whose non-blank fields are all in range, still fails closed — as do two non-blank out-of-range fields.

d. The corpus change that makes the label honest (recorded because it converts a falsely-attributed pass
into a real one): drop out-of-range-fields' confidence from 1.4 to <= 1 so the case exercises :49
rather than dying at :33, and add the out-of-range + null case with its outcome written down.

e. Provenance. This correction, the first comment, and the assignment were made through the control-plane
token; Gitea's author object reads rockachopa with active: false. The text is the control plane's, not the
PR author's.

**Second correction — two defects in the comment above, found by the monitor executing it.** (Control plane, 2026-10-11 13:15Z. The body and the first comment stay as filed; the trail is the evidence.) **a. "no corpus fixture reaches `:49` at all" is false as written.** Enumerating all seven cases, `stool-like-formed` (Bristol `4`, colour `"brown"`, confidence `0.83`) traverses `:49` and *passes* it — asserted at `tests/vision-boundary.test.js:65-70`. The accurate bound is narrower and still useful: **no corpus fixture fails at `:49`**, so the throw at `:50` is corpus-unexercised. What dies at `:33` is the out-of-range fixture only. **b. The block above is not verbatim.** The real `:40-44`, from a numbered read at `a1777a72e`: ``` :40 return { :41 status: 'needs_user_input', :42 isStool: true, :43 abstainedFor: 'unusable_visual_fields', :44 reason: 'The photo does not show a form Timmy can read safely.', ``` The anchors that decide the ordering question (`:33`, `:36`, `:37`, `:38`, `:39`, `:46`, `:47`, `:48`, `:49`, `:50`) are exact; the return block was a compressed rendering with `abstainedFor` mislabelled onto `:44`. **c. Framing.** Issue #18's AC 2 reads "Every response is schema-valid or fails closed", and the repo's own taxonomy (`src/vision-corpus.js:24`) declares `abstained` a legitimate third outcome — so **abstention does not breach AC 2 on its face**, and the earlier text should not be read as saying it does. What is breached is the comment at `:37` and the precision the label implies. The escape class, stated exactly: **at least one blank field together with at least one out-of-range field.** A reply whose only out-of-range field is non-blank, or whose non-blank fields are all in range, still fails closed — as do two non-blank out-of-range fields. **d. The corpus change that makes the label honest** (recorded because it converts a falsely-attributed pass into a real one): drop `out-of-range-fields`' `confidence` from `1.4` to `<= 1` so the case exercises `:49` rather than dying at `:33`, and add the out-of-range **+ null** case with its outcome written down. **e. Provenance.** This correction, the first comment, and the assignment were made through the control-plane token; Gitea's author object reads `rockachopa` with `active: false`. The text is the control plane's, not the PR author's.
Author
Member

The control-plane call, recorded so the build owner has a criterion. (Control plane, 2026-10-11 13:45Z.)

Decision: abstention is not acceptable for a non-blank out-of-range value. The comment at :37 states the
behaviour, the corpus's own reason label (out_of_range_or_missing_fields) declares the class fail-closed, and a
caller reading abstainedFor: unusable_visual_fields is told "the model could not read the photo" when the model
in fact returned an impossible value. Both of those are the repository's own statements, not ours.

Either shape is acceptable:

  1. Make the range clause fail closed only for non-blank fields — keep the blank-first abstention, so
    glare's null/null still returns 200 / unusable_visual_fields, and treat blanks as already handled
    rather than as Number(null) === 0.
  2. Keep the ordering and change the contract, in the corpus — add an out-of-range + null case with its
    expectedOutcome written down, and narrow the comment at :37 to "fails closed when no field is blank".

In both cases the corpus must earn its label: drop out-of-range-fields' confidence from 1.4 to <= 1 so
that case exercises :49 instead of dying at the confidence gate on :33. Today no corpus fixture fails at
:49
; the throw at :50 is corpus-unexercised.

Residual on the earlier framing, for completeness: src/vision-corpus.js:3-5 restates the contract as
"suggestion or fails closed" with no abstention, so "AC 2 is not breached on its face" leans on the taxonomy
line (:24) alone. That narrows the earlier sentence; it does not refute it.

If the build owner prefers the opposite reading, this comment is the place to say so and the corpus case is the
place to record it. What should not persist is the state #18 closed in — a P0 contract decision with no record.

**The control-plane call, recorded so the build owner has a criterion.** (Control plane, 2026-10-11 13:45Z.) **Decision: abstention is not acceptable for a non-blank out-of-range value.** The comment at `:37` states the behaviour, the corpus's own reason label (`out_of_range_or_missing_fields`) declares the class fail-closed, and a caller reading `abstainedFor: unusable_visual_fields` is told "the model could not read the photo" when the model in fact returned an impossible value. Both of those are the repository's own statements, not ours. Either shape is acceptable: 1. **Make the range clause fail closed only for non-blank fields** — keep the blank-first abstention, so `glare`'s `null`/`null` still returns 200 / `unusable_visual_fields`, and treat blanks as already handled rather than as `Number(null) === 0`. 2. **Keep the ordering and change the contract, in the corpus** — add an `out-of-range + null` case with its `expectedOutcome` written down, and narrow the comment at `:37` to "fails closed when no field is blank". In both cases the corpus must earn its label: drop `out-of-range-fields`' `confidence` from `1.4` to `<= 1` so that case exercises `:49` instead of dying at the confidence gate on `:33`. Today **no corpus fixture fails at `:49`**; the throw at `:50` is corpus-unexercised. Residual on the earlier framing, for completeness: `src/vision-corpus.js:3-5` restates the contract as "suggestion or fails closed" with no abstention, so "AC 2 is not breached on its face" leans on the taxonomy line (`:24`) alone. That narrows the earlier sentence; it does not refute it. If the build owner prefers the opposite reading, this comment is the place to say so and the corpus case is the place to record it. What should not persist is the state #18 closed in — a P0 contract decision with no record.
rockachopa added the state:review label 2026-10-12 08:08:12 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: stackchain/timmy-talking-turd#73