Skip to content

fix(schema): merge index-derived props with a declared key of the same name - #1659

Merged
ssalbdivad merged 1 commit into
mainfrom
fix/index-derived-props
Sep 19, 2026
Merged

ssalbdivad merged 1 commit into
mainfrom
fix/index-derived-props

Conversation

@ssalbdivad

@ssalbdivad ssalbdivad commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

When one side of a structure intersection rejects undeclared keys and the other has an index signature, the signature is narrowed to the declared keys and any single-key results become props. Those props were appended to the side they came from, where intersectConstraints never intersects two props of the same key, so

type({ a: "number", "[string]": "number" })
	.and({ a: "number", b: "number", "+": "reject" })

came out as { a: number, a: number, b: number, + (undeclared): reject }, a node with a duplicate key. The reduce-time duplicate check never saw it because the intersection result is built prereduced.

…e name

When one side of a structure intersection rejects undeclared keys and the
other has an index signature, the signature is narrowed to the declared keys
and any single-key results become props. Those props were appended to the
side they came from, where `intersectConstraints` never intersects two props
of the same key, so

	type({ a: "number", "[string]": "number" })
		.and({ a: "number", b: "number", "+": "reject" })

came out as `{ a: number, a: number, b: number, + (undeclared): reject }`, a
node with a duplicate key. The reduce-time duplicate check never saw it
because the intersection result is built prereduced.

The derived props now go through the intersection as right-hand operands, so
each one is merged with its same-key prop and the result is the same in
either operand order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kLpFgnvmw3sL4PreSwu7n

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • ark/schema/structure/structure.ts — index-signature narrowing now collects the extracted props into a derived array and appends them as right-hand operands (r) rather than concatenating them onto the side they originated from. Because intersectConstraints only merges a right operand against matching left operands, this routes each derived prop through a same-key merge; previously the second-branch derived props landed on l, where left props never intersect one another, leaving a duplicate key in the prereduced result.
  • ark/type/__tests__/objects/onUndeclaredKey.test.ts — regression test pins { a: number, b: number, + (undeclared): reject } for L.and(R) and asserts operand-order symmetry with R.and(L).expression.

I verified the test is not theatre: reverting structure.ts to the base revision makes it fail with { a: number, a: number, b: number, + (undeclared): reject }, exactly the duplicate-key node described. pnpm test passes all 1748 tests, and the derived props are always right-hand operands in both branches, so no same-side left/left collision can remain.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@ssalbdivad
ssalbdivad merged commit 0653b23 into main Sep 19, 2026
7 checks passed
@ssalbdivad
ssalbdivad deleted the fix/index-derived-props branch September 19, 2026 16:56
@github-project-automation github-project-automation Bot moved this from To do to Done (merged or closed) in arktypeio Sep 19, 2026
@ssalbdivad ssalbdivad mentioned this pull request Sep 24, 2026
4 tasks done
ssalbdivad added a commit that referenced this pull request Sep 24, 2026
Bumps all publishable packages and documents the changes merged since 2.2.4:

- preserve escaped backslashes in regex literals (#1634, @spokodev)
- fix recursive discriminated unions referenced from a Record (#1641, @xianjianlf2)
- prevent a crash validating unions of object arrays (#1638, @xianjianlf2)
- merge index-derived props with a declared key of the same name (#1659)
- preserve parameter labels when a type.fn implementation annotates an
  optional param (#1615, @aswinsvijay)

#1634 changes behavior for regex literals that contain `\\`: `"/\\\\d/"`
previously compiled to the digit class `\d` and now matches a literal
backslash followed by "d", matching the equivalent RegExp instance. The
changelog calls this out with a migration note.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ssalbdivad added a commit that referenced this pull request Oct 7, 2026
When one side of a structure intersection rejects or deletes undeclared
keys and the other has an index signature, the signature is narrowed to
the first side's keys and its single-key branches become props. They
came from `normalizeIndex`, which makes each key required, as a parsed
`Record<"a", t>` requires it. An index signature never requires a key,
so a narrowed one doesn't either: each prop it yields is now optional.

As required props they changed results (all pre-existing in 2.2.7):
- `type({ "+": "reject", "a?": "number" }).and({ "[string]": "number" })`
  was `{ a: number, + (undeclared): reject }`, rejecting `{}`, which
  both operands accept. It is `{ a?: number, + (undeclared): reject }`.
- With `"a?": "string"` it threw `Intersection at a of string and
  number`. It is `{ a?: never, + (undeclared): reject }`.
- A Disjoint at such a key wasn't marked optional, so a union could be
  discriminated on a key only its index branch constrains. Once the
  memo held `l & r` for `l = { "+": "reject", a: "string" }` and
  `r = { "[string]": "number" }`, `l.or(r)` was discriminated on `a`
  and rejected `{}` and `{ b: 1 }`, which `r` accepts (wave E's E26,
  the validity half; the operand order half is the next commit).

Where the other side requires the key, required and optional intersect
to required, so results there are unchanged (#1659's
`{ a: number, b: number, + (undeclared): reject }` included).

Behavior changes from 2.2.7, required by correctness: the three above.
New test: intersection.test.ts "narrows an index signature to optional
props" (fails before: `{ a: number, ... }`).

Gates: tsc clean; 1893 tests passing, cyclicGenerated timing out at
its 10 s limit under load 19 (it passes alone in 13 s). Parity
battery identical. Not measured: the change runs only
when an undeclared side meets an index signature its keys narrow.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ssalbdivad added a commit that referenced this pull request Oct 7, 2026
The intersection memo answered `r & l` with the cached result of
`l & r`, its Disjoint inverted. That is sound only if every intersection
is anti-symmetric, and a structure's wasn't. Props an index signature
narrows to on l's side were intersected as r operands, after r's own
constraints, so they met r's props from the right: with
`l = { "+": "reject", a: "string" }` and `r = { "[string]": "number" }`,
a fresh `r.and(l)` threw "Intersection at a of string and number",
naming l's operand first, and "number and string" once `l.and(r)` had
run. Before the previous commit, the same history decided whether
`l.or(r)` was discriminated on `a` (wave E's E26, pre-existing in
2.2.7).

- `intersectOrPipeNodes` no longer looks up the reversed key: `r & l`
  is computed and memoized under its own.
- A structure intersection lists l's derived props ahead of r's
  constraints. `intersectConstraints` meets each r operand with every
  accumulated l operand in turn, so l's derived props merge with l's
  own prop of the same key (or join the l list) before any of r's
  constraints, which then meet them as l operands. They still can't
  start in the l list, where a prop of the same key would never be
  merged with them (#1659). r's derived props stay after r's
  constraints.

Order independence: the algebra differential (57 definitions, every
ordered pair through and/or/extends/overlaps/exclude/pipe and the
union's discriminant, 19,139 lines) gives the same lines run in forward
and in reversed pair order. At the branch tip 6 lines differed, after the
previous commit 4, now 0.

Behavior changes from 2.2.7, required by correctness:
- A Disjoint at a key an index signature narrows on l's side names l's
  operand first in either order:
  `type({ "[string]": "number" }).pipe({ "+": "reject", a: "string" })`
  throws "Intersection at a of number and string" (was "string and
  number", the only line of the differential that changes).
- Such a Disjoint is found when r's prop is met rather than after all of
  r's constraints, so where another key also conflicts, a different one
  may be reported: `type({ b: "string", "[string]": "string" }).and({
  "+": "reject", a: "number", b: "number" })` reports `a` where 2.2.7
  reported `b`. Both keys are unsatisfiable.

New test: intersection.test.ts "orients a Disjoint by its operands in
either order" (fails before: `r.and(l)` names "string and number").

Gates: tsc clean; 1894 tests passing, cyclicGenerated timing out at its
10 s limit under load 20+ (passes alone in 15 s). Parity battery
identical.

Creation (source, a fresh process per cell, arms interleaved, n=20,
load 21-25), against the previous commit: object 1.11x, and 1.02x,
union 1.00x, indexed 1.04x, extends 0.98x, scope 1.00x, 40-branch
tagged union 0.95x; every sign p >= .26, so no change is detected
either way. A miss no longer builds the reversed key; a hit that was
reversed is recomputed once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done (merged or closed)

Development

Successfully merging this pull request may close these issues.

1 participant