Skip to content

test(drift): redundant field_value_or_absent lint#889

Merged
bokelley merged 3 commits into
mainfrom
bokelley/redundant-tolerance-lint
Apr 24, 2026
Merged

test(drift): redundant field_value_or_absent lint#889
bokelley merged 3 commits into
mainfrom
bokelley/redundant-tolerance-lint

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #876's review pass: warn when a storyboard asserts `field_value_or_absent` against a path the response schema already marks required. The tolerance is meaningless there — the spec guarantees the field is present, so the "absent" branch is dead code, and authors should use plain `field_value`.

Test-only change. No library code touched, no changeset.

Mechanism

  • New helper `isPathRequired(schema, segments)` mirrors the existing `isPathReachable` in `test/lib/storyboard-drift.test.js`. Walks a Zod v4 schema along the parsed path; returns `false` when any segment traverses a tolerance wrapper (`optional`, `nullable`, `default`, `catch`), a `record` (no key guaranteed), or a union branch that omits the field. Conservative on unions (required iff EVERY branch requires it), aggressive on intersections (required if EITHER side requires it — same symmetry as `isPathReachable`).
  • New describe block walks every storyboard's `field_value_or_absent` assertions and fails when the path is schema-required. Zero entries today (no storyboard uses the matcher yet); engages automatically as adoption grows.
  • 10 unit tests pin `isPathRequired` semantics against inline Zod schemas — required string, optional, nullable, default, nested required object, intermediate optional wrapper, missing field, array element, record, union (both branches / only one).

Why test-only is the right surface

The review proposed a runner-time warning channel, but the runner has no cross-step warning surface today and nothing currently consumes one. Piggybacking on `storyboard-drift.test.js` is the right fit: it's already where storyboard paths meet Zod schemas, it already has an allowlist pattern (`UPSTREAM_SCHEMA_DRIFT`), and a CI test with an allowlist is a warning-shaped gate (fails the build, stays human-decidable). Adding a net-new runner warning channel + plumbing just for this check is speculative — revisit if other lint rules surface.

Test plan

  • `node --test test/lib/storyboard-drift.test.js` — 399 pass / 4 skipped (pre-existing upstream drift)
  • `npm run build` clean
  • No storyboards break — the matcher isn't used in any existing storyboard yet (the spec-repo update at Document field_value_or_absent in storyboard-schema.yaml check enum adcp#3030 is the prerequisite for re-adding the `replayed` assertion that motivates it).
  • CI green

Related

🤖 Generated with Claude Code

bokelley and others added 3 commits April 24, 2026 10:29
… fields

Follow-up to #876 review. When a storyboard asserts
`field_value_or_absent` on a path the response schema already marks
required, the tolerance is dead code — the spec rules out the absent
branch. The new `isPathRequired` schema walker + describe block catches
this at the same gate as the existing reachability lint, so authors
get redirected to plain `field_value` when tolerance is meaningless.

Conservative on unions (required only if every branch requires it),
aggressive on intersections (required if either side requires it) —
matches the symmetry of `isPathReachable`. Covers 10 unit-test cases
on the helper + a storyboard-wide gate that's empty today (no
storyboards use the matcher yet) but engages as adoption grows.

Pure test-file change, no library code touched — no changeset.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…essage

Review nits from code-reviewer pass on PR #889:
- Intersection handling is correct semantics (a value must satisfy both
  sides), not a 'symmetry' hack. Rewrite the doc comment.
- Quote `field_value` in the lint failure title so CI log grep finds
  both sides of the matcher pair.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 0ba2bc3 into main Apr 24, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant