Skip to content

test(cycle): the walk asserts the refusals it collects - #702

Merged
JArmandoAnaya merged 2 commits into
mainfrom
test/cycle-refusal-assertion
Aug 19, 2026
Merged

test(cycle): the walk asserts the refusals it collects#702
JArmandoAnaya merged 2 commits into
mainfrom
test/cycle-refusal-assertion

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

The cycle walk collected every API call the application made that the API refused into a Set, and read that set exactly once — to suppress a console error whose location matched a member. Nothing asserted it. Its two neighbouring collectors are both asserted in the walk's final step, badRequests against an empty array and abortedApiCalls against an exact two-element list, so apiRefusals was the only one of the three that existed solely to make an assertion pass, and the only one that grew without bound. A genuine 500 in a walked route reached the suite as the same thing as an expected 404: added to the set, exempted from the console assertion, never looked at, and the step titled "the whole walk produced a clean console" reported a clean console.

This adds a second collection alongside the set. The set stays keyed by URL, because that is what a console message's location carries and the suppression genuinely needs it; the new refusedApiCalls list is keyed by method, path and status, because that is what a person can check, and it is pinned in the final step with an exact toEqual.

The conditional entry, and why it is read rather than assumed

Six of the seven expected refusals are 404s for documents that do not exist yet, and they are the same on every installation. The seventh is not. GET /inference/download-size prices the model the connection form seeds itself with, and reading that price needs the optional local-inference runtime, so it answers 500 wherever the extra is absent — which is what the browser cycle (chromium) job installs — and 200 wherever it is present.

Rather than guess at the environment, the walk reads what the form rendered. DownloadSizeLine shows size-known or size-unavailable, and the expectation splices the 500 in exactly when the second one is what appeared. That keeps the entry bounded rather than merely conditional: the method, path, status and position stay pinned literals, so a probe that started answering 404 or 422 would still render size-unavailable, still set the flag, and still fail the assertion.

Making that readable required one behavioural change to the walk. The seeded model's probe previously raced the walk's own typing — on one machine it fired as the dialog tore down after submit rather than when the dialog opened — so where the refusal landed, and whether it landed at all, was timing. The walk now waits for the size line before touching the model select, which orders the probe without changing what is under test.

Test plan

The gate for this change is the walk failing when it should. Four runs, in order, each on a base install where the curated model's probe genuinely answers 500:

Run State Result
A Unmodified spec, a real 500 already occurring 1 passed (19.8s) — the defect
B Collector added, no assertion yet 1 passed (21.5s) — the defect, amplified
C Assertion added, inference_download_size temporarily raising for every model 1 failed
D Temporary raise reverted 1 passed (24.1s)

Run C is the one that matters, and it named the route and the status:

    Error: expect(received).toEqual(expected) // deep equality

    - Expected  - 2
    + Received  + 7

        "GET /inference/download-size 500",
    -   StringMatching /^GET \/projects\/[0-9a-f-]+\/schema\/drafts\/annotation 404$/,
    -   StringMatching /^GET \/projects\/[0-9a-f-]+\/schema\/drafts\/annotation 404$/,
    +   "GET /inference/download-size 500",
    +   "GET /inference/download-size 500",
    +   "GET /projects/df68a16c-808e-4f5a-a300-27f37e28174a/schema/drafts/annotation 404",
    +   "GET /projects/df68a16c-808e-4f5a-a300-27f37e28174a/schema/drafts/annotation 404",
    +   "GET /inference/download-size 500",
    +   "GET /inference/download-size 500",
    +   "GET /inference/download-size 500",
      ]

The temporary raise was never committed, and the diff touches one file.

Both halves of the inference matrix were run against the same pinned array. uv sync --locked produces the installation the browser cycle job installs, and there the list is seven entries with the 500 at index 4. uv sync --locked --extra local-inference produces the other half, and there the flag reads false, the entry is spliced out, and the list is six. Neither half required a different array.

Full gate on the rebased branch:

3970 passed, 16 skipped in 33.62s
All checks passed!
Success: no issues found in 173 source files
frontend/annotator test:       Tests  1062 passed (1062)
frontend/ui-core test:       Tests  1149 passed (1149)
  278 passed (40.1s)
  1 passed (27.0s)
All checks passed.
FULL_CHECK_EXIT=0

One axis this branch cannot verify

The assertion has never executed on a CI runner, because it did not exist until this branch and this branch has not run there. Two mechanisms could legitimately produce a different exact list on a slower machine, and both are measurement rather than regression if they appear. The autosave debounce in SchemaTab is 400 ms and the draft write populates the cache rather than invalidating it, so whether a given GET .../schema/drafts/curated answers 404 or 200 depends on whether that timer fired before the walk's tab switch and reload; if it lands earlier on a runner, the list is short by one. Separately, the active schema and the curated draft are requested in the same render, so they complete in whatever order the server's threadpool finishes them, and the pair could invert.

If the browser cycle job goes red on either, the repair is to re-measure and pin what it reports. Widening the assertion — expect.arrayContaining, sorting both sides, or dropping the conditional entry — would restore precisely the blind spot this change closes. Worth noting that the cycle config sets one retry in CI, so an inversion would surface as a flaky pass, which must not be read as a pass.

Found, not fixed

A full gate run on this branch failed at exit 1 while all 1149 ui-core tests passed, because a @radix-ui/react-focus-scope teardown timer fired after jsdom had torn down the document that owned its element. It did not reproduce in isolation on this branch or on unmodified main at da3304a, both of which came back clean at exit 0, so it is a contention-sensitive race rather than a standing red or anything this branch introduced — the diff here is one Playwright spec that the ui-core vitest run never loads. It is filed with its stack and both reproduction attempts as the focus-scope teardown race (#701) and is not addressed here.

The seeded model's probe fires once more than it needs to, when the create dialog resets to its curated default as it closes after a connection is submitted. It is harmless, it predates this branch, and pinning the list documents it rather than changing it.

Closes #697

The pinned-list comment attributed the annotation-draft 404s to the schema
editor and said the second read of each pair follows a write; both are
wrong. The pair comes from the annotator's add-a-class dialog, gated on
addingClass, and the second read is a remount or page.reload() — a saved
draft is written straight into the cache rather than invalidated, so a
write never triggers a refetch on its own.

The size-line wait relied on connection-model being visible to guarantee
the seed landed on a curated model, but the seeding effect falls back to
Custom model with an empty id when the catalog offers nothing that answers
a point prompt, and DownloadSizeLine renders nothing for an empty id — so
that installation would die on a bare 15-second timeout with nothing to
explain it. Add an explicit assertion that the select shows a curated
model before waiting on the price probe only a curated model triggers.

Also trims the apiRefusals comment from four sentences to three, cutting
an opening clause that only summarized what a later sentence already said
properly.
@JArmandoAnaya JArmandoAnaya added frontend annotator / ui-core / app packages ci CI, tooling, repo automation tooling Developer tooling: scripts, checks, local workflow labels Aug 19, 2026
@JArmandoAnaya
JArmandoAnaya merged commit d2a37f5 into main Aug 19, 2026
15 checks passed
@JArmandoAnaya
JArmandoAnaya deleted the test/cycle-refusal-assertion branch August 19, 2026 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI, tooling, repo automation frontend annotator / ui-core / app packages tooling Developer tooling: scripts, checks, local workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The cycle walk never asserts the API refusals it collects, so a real one cannot fail the run

1 participant