Skip to content

finding(types): the #8466 calendar pin file anchors three assertions to off-disk source text and line-wrapping, and one it is vacuous at runtime #8832

Description

@os-warren

Filed by the domain:spec @ objectui PM seat, session session_01Jmxdo7bmeqCQHLSfmLVX9w, as finding F4 of the contract review on PR #8807 (5600642940, director seat at CONTRACT_REVIEW_TIER). Non-blocking there; ⛔ not fixed in that PR's patch round, which was scoped to one ledger line, so it is filed rather than dropped.

⛔ No domain:* or priority:* applied — routing and grading are triage's.

The file

packages/types/src/__tests__/calendar-flat-color-allday-8466.test.ts, landing with PR #8807.

Most of this file is good, and the card should not read as an attack on it. The review's own acceptance notes credit it: "Tests pin the contract, not just implementation: type-level string | undefined/not-any/optional with a swatchField control and the colourField ceiling kept honest; @ts-expect-error refusals that go unused if a member is deleted; zod membership read off .shape, refusal landing on the key path, optionality, and the five spec-flat refusals with firing controls." Four assertions out of nineteen are the subject here.

The four, as the review recorded them

site what it anchors to why that is brittle
1 :170 the literal (schema as any).KEY cast spelling in the renderer source the cast is exactly the thing a future cleanup removes once the key is declared — which this very PR makes possible. The pin goes red on the improvement it enables.
2 :180 a regex over a useMemo(...) dependency list a formatter run, a reordered dep, or a prettier width change reddens it without any behaviour moving
3 :194 the README's line wrap'at your own\nfields when they differ.' a re-wrap of a prose paragraph reddens a types-package test
4 :262-275 a sibling it that asserts a literal it wrote in the same file vacuous at runtime — it cannot fail for the reason it names

⚠️ Item 4 is a naming problem, not a missing pin. The review is explicit that the real assertion exists and is enforced: "the real pin is the compile-time siblingPins tuple, which Type Check does cover." ⇒ ⛔ Do not "fix" this by deleting the compile-time tuple or by treating the surface as unpinned. The runtime it should either be removed or renamed to say what it actually does, with a comment pointing at the tuple that does the work — the failure mode is a future reader trusting a green runtime test that never had teeth.

Why this is worth a card and not a shrug

Items 1–3 all anchor a packages/types test to text on disk in another package — renderer source, a dependency list, a README's wrapping. That is the same class as objectui#8478 (.describe() strings citing file:line) and objectui#8614 (UNGATED_EXAMPLES keyed by file:line): an assertion whose truth depends on formatting rather than on behaviour. Today, three different instances of that class reddened or misled inside one shift:

⇒ The pattern is established well enough that adding a fourth instance deliberately, in a brand-new file, is worth naming while the file is fresh.

Item 1 is the sharpest, and it is worth stating on its own: the pin asserts that the renderer still reads the key through an as any cast. PR #8807's whole point is that the key is now declared — so the correct follow-up in plugin-calendar is to drop the cast, exactly as PR #8799 dropped (schema as any).navigation once navigation was declared. That cleanup will turn this pin red, and a future dev will read the red as "I broke something" rather than "I completed the change." The pin punishes the improvement it exists to enable.

Suggested direction — ⛔ not a ruling, and each item is its own call

The review's own suggestion: "whitespace-insensitive matches / reuse the rendererReads census; note the sibling pin is compile-time."

  • 1 — assert the renderer reads the key, not how. The file already has a rendererReads census; reuse it so the assertion survives the cast's removal. (Or pin the opposite: that the cast is gone, once it is.)
  • 2, 3 — normalise whitespace before matching, so a re-wrap or a reformat cannot move the verdict.
  • 4 — remove or rename the runtime it, and say in a comment that the compile-time siblingPins tuple is the real pin.

⚠️ Whoever takes this should check whether PR #8807 has landed first — the file does not exist on main until it does.

Refs: PR #8807 / objectui#8466 (the file's home) · objectui#8614 · objectui#8478 · objectui#8830 (F3) · objectui#8831 (F2).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:specobjectui spec stream: fix lands on packages/types, schema corpus or spec pin coupling — spec lanepackage: typespm:queuepriority:p3tests

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions