Skip to content

fix(scripts): parseParamNames strips the full TypeScript modifier run, so a parameter property is no longer read as readonly - #13292

Draft
os-elon wants to merge 2 commits into
mainfrom
claude/issue-13227-parseparamnames-modifiers
Draft

fix(scripts): parseParamNames strips the full TypeScript modifier run, so a parameter property is no longer read as readonly#13292
os-elon wants to merge 2 commits into
mainfrom
claude/issue-13227-parseparamnames-modifiers

Conversation

@os-elon

@os-elon os-elon commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13227

parseParamNames in scripts/check-dispatcher-error-vocabulary.mjs cleaned a parameter with a single anchored alternation carrying a g flag. The flag reads as "strip them all", but ^ with no m matches at position 0 once, so exactly one modifier came off: private readonly code: string parsed as a parameter literally named readonly.

What that costs — measured, not assumed

helperCodesFor does parseParamNames(decl.params).indexOf(ident). A wrong name means -1, the identifier is never recognised as a parameter, and the whole helper is dropped: no site AND no unresolved. Same silent-drop class as #9223 / #9460 / #10918 / #13131, one layer further in.

Through the real deriveSites, before (at 74049254d4) and after, with the single-modifier positive control beside it so the zeros are readings rather than a dead harness:

specimen before after
A constructor(private readonly code: string, readonly message: string) sites: [] unresolved: [] 1 codehelper site, PARAM_PROP_CODE
C private override readonly code (three modifiers) sites: [] 1 site, THREE_MOD_CODE
B control: readonly code (one modifier) 1 site 1 site, unchanged
D control: a parameter literally NAMED readonly 1 site 1 site, unchanged
E control: a parameter literally NAMED override 1 site 1 site, unchanged

The modifier set, enumerated from the grammar

The card's example is two modifiers. The real maximum is three, and the order is fixed. Measured against this repo's own TypeScript 6.0.3 — override needs a class that extends another, or the compiler reports it as a class error and the parameter question never gets asked:

ACCEPTED   private readonly x        public readonly x        protected readonly x
ACCEPTED   override readonly x       private override readonly x
ACCEPTED   public override readonly x
REJECTED   readonly public x     -> 'public' modifier must precede 'readonly' modifier
REJECTED   readonly override x   -> 'override' modifier must precede 'readonly' modifier
REJECTED   static x / abstract x -> modifier cannot appear on a parameter

So the run is (public|private|protected)? override? readonly?, encoded in that order rather than looped over a set — a "strip any word in this set, repeatedly" loop accepts spellings TypeScript does not, which is the match-everything direction. in / out are TYPE-parameter modifiers and never reach the value-parameter slice enclosingDeclaration hands over.

Why this stayed textual instead of reading the AST

typescript is a declared root devDependency and two sibling root gates already import it, so the AST route was genuinely available. It is the wrong one here, and the reason is measured rather than inherited from the file's "why textual" header.

enclosingDeclaration's DECL_HEADER_RE also matches const x = someCall(. Of the 7646 slices this function is handed on packages/** non-test source, 1218 are not a valid parameter list at all — they are argument lists — and on 1072 of those a recovering TypeScript parse invents MORE THAN ONE parameter: authService as any becomes three confident parameters named authService, as and any; await res.json() becomes await, res, json. Those names are exactly what indexOf(ident) searches. An AST route would therefore MANUFACTURE the wrong-index hazard the card only warns about, across a thousand slices, where the textual reader degrades to one bad name. The over-matching header regex is #13226's subject and is untouched here.

Correctness, against a real oracle

Every parameter list the gate reaches on packages/** non-test source, compared with the TypeScript AST, restricted to slices that really are parameter lists:

valid parameter lists                              5979
  BEFORE agrees with the AST                       5868  (98.14%)
  AFTER  agrees with the AST                       5919  (99.00%)
  lists whose parsed names changed                   54
  REGRESSIONS (was AST-correct, now is not)           0

The 60-list residue is a different defect in splitTopLevel — the > of => decrements bracket depth — filed as #13291 and deliberately not fixed here.

Blast radius on this tree

The full gate run is byte-identical before and after: 22 unregistered code-stamping sites, all classified, 1 awaiting a ledger entry. 51 declarations are now read correctly, but none of them carries a code stamp assigning one of its parameters, so no verdict moves and no new red is possible. This is prevention, consistent with the p2 grade. The nearest live shape, MetadataError's constructor(public readonly code: string, message: string) in packages/metadata-core/src/errors.ts, now parses as ["code","message"]; all four codes it carries are already registered.

Pins

--self-test grows from 175 to 194 assertions: every run length TypeScript admits, plus a NEGATIVE set for parameters genuinely named after a modifier word — override is a real parameter name three times in this repo's own source — plus the end-to-end pair through deriveSites with its control.

Reverse-verified from the committed state: restoring only the old one-shot strip while keeping the new pins turns 9 assertions red, naming the exact mis-parses, while every positive control and every other negative case stays green. Mutation confirmed on disk by marker counts and a git hash-object change; restore confirmed by the blob hash matching HEAD with an empty git diff HEAD.

Local verification, at ae72953fdf

node scripts/check-dispatcher-error-vocabulary.mjs --self-test
  -> 8 shapes + 194 assertions OK (vocabulary + #9098 door typing)
node scripts/check-dispatcher-error-vocabulary.mjs
  -> OK - 22 unregistered code-stamping site(s), all classified; 1 awaiting a ledger entry (#8846)
pnpm lint  (eslint . --no-inline-config, whole repo)                    -> exit 0

Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (1 changed path), all green:
check:agent-test-spelling · check:bash32-floor · check:cli-command-ids · check:cross-package-test-inputs · check:dispatcher-error-vocabulary · check:entry-guard · check:parse-guard · check:pnpm-filter-targets · check:watch-hint-literal · check:nul-bytes · check:pm-dispatch-gates · check-self-test-wired.mjs --self-test and its scan · check-ci-filter-parity.mjs · check-cross-package-test-inputs.mjs · check-shard-attestation.mjs · pm/bare-root-worklist.mjs --self-test.

check-test-completeness.mjs is NOT MEASURED locally: with no argument it exits 3 on its own PREREQUISITE NOT MET branch, which its failure text states is not a red and not a finding — it grades a saved turbo run test log that only CI produces.

Declarations

Out of scope, filed instead of fixed: #13291.


Generated by Claude Code

claude added 2 commits August 29, 2026 17:36
`private readonly code: string` parsed as a parameter named `readonly`:
the strip was one anchored alternation carrying a `g` flag, and `^` with
no `m` matches at position 0 once, so exactly one modifier came off.
`helperCodesFor`'s `indexOf(ident)` then answered -1 and the whole code
helper was dropped in silence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw
Positive cases at every run length TypeScript admits (up to three:
accessibility, override, readonly), negative cases for parameters
genuinely NAMED after a modifier word, and the end-to-end pair through
deriveSites with the single-modifier positive control beside it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants