Skip to content

fix(lint): say in the *When skip trailer that a skip is not a to-do item - #11708

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-11673-skip-reason-triageability
Aug 24, 2026
Merged

fix(lint): say in the *When skip trailer that a skip is not a to-do item#11708
os-steve merged 1 commit into
mainfrom
claude/issue-11673-skip-reason-triageability

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #11673

Route A from the card: the trailer sentence only. The second half — per-entry
source context — is recommended against, on measured grounds set out below. That
recommendation is the PM's to ratify, which is why this says Part of rather than a
closing keyword.

What changed

packages/lint/scripts/check-doc-formula-expressions.mjs, one file, +63 lines:

  1. The trailer in renderFieldRuleSkips() now says a skip is not a to-do item, that
    every reason above answers "why could this scan not read a layer here?" and never
    "what layer does this fragment document?", and that the layer must be read off the
    document before anyone re-authors a listed site.
  2. Two self-test pins, because the trailer is a string nobody else reads — deleting it
    breaks no other assertion in this file and turns no gate red anywhere:
    • the trailer states both load-bearing halves;
    • no rendered entry ever NAMES a layer the gate did not derive. This is the guard on
      the fix itself. The cheap version of this card (scrape the nearby comment, print
      PageComponent) is easy to write and exactly wrong, and it would be believed.
  3. A TSDoc block on the renderer recording why the trailer exists and why it is a trailer
    rather than per-entry context.

Zero behaviour change. The discriminator is untouched, no verdict moved, no count moved.

Measurements

Re-measured at base ffbb7a100, not quoted from the report body (which predates #11672):

✓ check:doc-formula-expressions (field-level `*When`, #11407): 14 predicate(s) on a
statically determinable field layer judged clean; 6 skipped as undeterminable.

Identical before and after. Self-test 48 → 50 cases passed (the two new pins).

Non-vacuity, both directions, predicted before running

Both ablations ran under trap … EXIT INT TERM; both restores verified byte-identical
by sha256 against the pristine copy. No rebuild leg applies — the mutated artifact is the
.mjs node executes directly, not a dist/ (the gate imports@objectstack/lint/dist,
but the script itself is never resolved through it).

ablationpredictedobserved
delete the trailer sentencepin 1 red✗ REPORT — the trailer says a skip is NOT a to-do item …, self-test 1 case(s) failed, exit 1
make the renderer print this fragment documents the PageComponent layer per entrypin 2 red, pin 1 green✗ REPORT — no rendered skip entry names a layer the gate did not derive; pin 1 stayed

Mutation proven on disk each time by anchored marker counts and sha256 change — the
edit script asserts its anchor count, so a zero-hit replace exits non-zero rather than
reporting a healthy no-op. Ablation B's pin-1-stays-green row is what shows the two pins
are independent, i.e. that pin 2 really is the claim guard and not a second copy of pin 1.

Why the second half is recommended against

The card's hypothesis was that printing the two source lines above each site would let a
reader triage the list without opening each file. Checking the other three skips falsifies
it, and the counter-evidence is stronger than "it doesn't always help".

The layer-stating comment does not generalise — it is 3 of 6, all in one file.

sitetwo lines abovestates the layer?
layout-dsl.mdx:821// e.g. on a PageComponent — …yes
layout-dsl.mdx:824// e.g. on a FormSection / FormField — …yes
layout-dsl.mdx:863// On a PageComponent, an app/nav entry, or a per-option visibleWhen:yes
flows.mdx:320{ name: 'opportunityName', label: …, type: 'text', / required: true,no
pages.mdx:165onClick: "navigate_to(…)" / },no
views.mdx:415required: true, // Override required / span: 'full', // …no

And for flows.mdx:320 the two-line window is actively misleading, in precisely this
card's own failure direction. It hands the reader { name, label, type, required } — the
shape #11651's ruling called "probably a field def", about which it said "probably is
exactly what a layer discriminator must refuse". The site is a flow-screen field whose bare
reference is correct. Route B would move that guess out of the gate, which is forbidden to
make it, and into the reader, who is under no such discipline. views.mdx:415 reads the
same way.

The decisive evidence is that per-entry context was already available and did not work.
#11651's own report body quoted the layer comments above :821 and :824 verbatim —

Their layer is stated only in a prose comment above each line: // e.g. on a PageComponent, // e.g. on a FormSection / FormField.

— and filed both sites under "re-authorable" in the same document. The author had the
context in hand, published it, and still misclassified. So the failure was never an
information deficit; it was a framing error, and route B reprints information that has
already been demonstrated insufficient. The trailer addresses the thing that actually broke.

Verification

All at final HEAD 4438ef8dc. Gate families derived, not recalled:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack — provenance line
confirmed "gate list derived from the tree of 'objectstack-ai/objectstack' at commit
ffbb7a1 … --repo … checked against this checkout's 'origin' remote — it holds"
, change
set "1 path(s) vs merge base … three-dot semantics". 7 families matched; all run locally,
all green, exit codes captured by redirect-then-capture before any pipe.

familyverdict line
check:doc-formula-expressions✓ … 14 predicate(s) … judged clean; 6 skipped as undeterminable. · ✓ … self-test: 50 cases passed
check:published-files✓ … 69 publishable package(s) of 78 workspace member(s) …
check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new …
check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned …
check:type-source-resolutioncheck-type-source-resolution OK — 77 packages with a tsconfig.json scanned …
check-plugin-teardown-shape.mjs✓ … 63 Plugin implementation(s) across 4597 source(s) …
check-affected-docs.mjs✓ affected-docs self-test: 395 cases pass.

Also check:nul-bytes (any edit): check-nul-bytes: OK (scanned 6536 text file(s) … no raw ASCII control bytes), plus a control-byte self-scan of the edited file with a positive
control proving the scanner fires.

pnpm lint — declared narrowing, not a skip. Ran eslint --no-inline-config --format json on the changed file: 1 file linted, 0 errors, 0 warnings, count read from the JSON
rather than asserted. Positive control: linting packages/lint/scripts/ yields 2 files and
the changed file is one of them, so the clean reading is a measurement and not an empty
population. Invariance for untouched files is read off the repo's own config, which states
it "never enables type-aware linting (no parserOptions.project, no typed
@typescript-eslint rules) for ANY file" — so a one-file diff cannot move any verdict on a
file it does not touch. CI runs the repo-wide scan regardless.

NOT MEASURED: CI conclusions — this PR and its report are delivered at draft-PR time
per the standing contract.

No changeset — checked against the rule, not assumed

changeset-check in pr-automation.yml has no path-based exemption: it counts
git diff --diff-filter=A … '.changeset/*.md' for every PR, and the only two escapes are
the skip-changeset label and the changeset-release/main PR. So the label is the
mechanism, and the question is only whether this PR publishes anything. It does not:
@objectstack/lint declares files: ["dist","README.md","CHANGELOG.md"], and
npm pack --dry-run lists 17 entries with zero under scripts/. The changed file ships
to no consumer; it is CI-only tooling. skip-changeset applied.

Generated by Claude Code


Generated by Claude Code

The skip list prints, per entry, why the PARSER could not read a layer. It
never says what layer the fragment documents, and those are different
questions: only the second decides whether a site could be re-authored.
Read as a worklist, the list produced a measurably wrong triage. Three
independent passes partitioned seven skips 4 re-authorable / 3 permanent;
judging the sites first gave 1 / 6, and one of the "re-authorable" four was
byte-identical to a skip the same ruling protected by name as a false red on
correct documentation.
Adds the framing sentence to the trailer and pins it, plus a guard that no
rendered entry ever NAMES a layer the gate did not derive. Verdicts, counts
and the discriminator are untouched: 14 admitted / 6 skipped on both sides.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@os-steveClaude

Copy link
Copy Markdown
Collaborator

ACCEPT — and the ruling on your open question is A. Route B is declined, permanently, on your evidence.

This is the outcome I asked for and did not expect to get. I wrote "I would rather be told the second half is unnecessary than get it built dutifully." You measured it and told me. Three separate arguments, and each would have been sufficient:

1. The decisive one — context was already available and did not work

#11651's report body quotes the layer comments above :821 and :824verbatim … and files both sites under "re-authorable" in the same document.

The author had the context in hand, published it in the same artifact, and still misclassified both. So the failure was never an information deficit, and route B reprints information already demonstrated insufficient. That is a fact about a closed card which cannot change, which is what makes the decline permanent rather than provisional.

2. Route B's window is actively misleading at exactly the sites it would serve

Verified independently:

flows.mdx:318-320 { name: 'opportunityName', label: 'Opportunity Name', type: 'text',
required: true,
visibleWhen: 'createOpportunity == true' },
views.mdx:413-415 required: true, span: 'full',
visibleWhen: "record.status != 'cancelled'",

Both read as object-field definitions. Neither is oneflows.mdx:320 is a flow-screen field whose bare createOpportunity reference is correct, and views.mdx:415 is a fields:array entry. So for two of the three sites that carry no layer comment, route B hands the reader the exact { name, type } shape #11651's ruling named as "probably a field def" and refused to admit.

⭐ Your framing of why that is disqualifying is the part worth keeping:

Route B moves that guess out of the gate, which is forbidden to make it, and into the reader, who is under no such discipline.

It does not eliminate the guess. It relocates it to someone with no rule binding them. I did not see that when I wrote the option, and it inverts my assumption — I took for granted that more context helps, and it misleads precisely where the gate cannot help.

3. The comment is 3 of 6, not the pattern

Confirmed: only the three layout-dsl.mdx sites carry a layer comment. flows.mdx:320, pages.mdx:165, views.mdx:415 have none. A feature that works for half its population and misleads on the rest is not a feature.

The fix itself

Route A only, one file, and pinned twice — which is what makes it more than a comment:

  • Pin 1 guards the trailer, "because the trailer is a string nobody else reads: deleting it breaks no other assertion and turns no gate red." Exactly right; an unpinned prose fix is a fix with a half-life.
  • Pin 2 guards the fix against its own cheap version: no rendered entry may ever name a layer the gate did not derive. So the forbidden implementation — scrape the comment, print "PageComponent" — goes red in the self-test rather than shipping. That is my ⛔ turned into a mechanism instead of a rule someone has to remember, and it is strictly better than what I asked for.

⭐ Ablation B is the detail that shows the pins are real: injecting the forbidden per-entry claim reddens pin 2 while pin 1 stays green. Two pins that always fail together are one pin with extra steps; these are independent.

Counts re-measured at your own base (14/6, not the report's 13/7) and identical across the change, since no verdict moved. Discriminator untouched.

Your refinement of my framing is accepted, and it is the better statement

the reason texts were the OCCASION, not the whole cause — the operative cause is framing (reading the list as a worklist), which is exactly what the trailer addresses and what per-entry context does not.

I had said the reason texts caused the misclassification. They did not; the layer comments for two of the three misread sites were already quoted in the report that misread them. What the reasons did was make a worklist reading feel safe. The trailer attacks the reading; context would have attacked the symptom.

Ruling: A — close #11673 on this PR

Not C. Your durability argument decides it:

the renderer's TSDoc now records why the fix is a trailer and not per-entry context, so the reasoning sits next to the code a future editor would touch.

A card can be closed and forgotten; a PR body is unreachable from the code. A TSDoc on the renderer is read by the exact person who would otherwise reimplement route B. That is the right home.

I will close #11673 by hand once this lands — the PR is Part of, and I am not spending a CI cycle on a one-word body edit.

Flipping ready; arming once every check is green. ⚠️ Note for the record: this seat has now been caught twice today arming-adjacent to a red (#11667, #11675), both times after flipping ready — so ready here means ready, not armed.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 24, 2026 13:39
@os-steve
os-steve added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit c86d86aAug 24, 2026
35 checks passed
@os-steve
os-steve deleted the claude/issue-11673-skip-reason-triageability branch August 24, 2026 14:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-steve@claude