Skip to content

Give surviving anonymous structs a synthetic name - #81

Merged
nx10 merged 2 commits into
mainfrom
fix/anonymous-struct-naming
Jul 28, 2026
Merged

Give surviving anonymous structs a synthetic name#81
nx10 merged 2 commits into
mainfrom
fix/anonymous-struct-naming

Conversation

@nx10

@nx10 nx10 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Problem

A multi-field anonymous struct (a sequence node with no meta.name) was named by getName, which via findDeepName steals the struct's first field name. That name then collides with the field itself, producing doubled access paths - e.g. the FSL bet shape yielded params.fractional_intensity.fractional_intensity.

Fix

  • Add freshStructName to NamingStrategy: a synthetic structN (backed by a dedicated counter) that dodges the struct's own field keys. The genuine-struct and empty-struct-with-outputs branches now name their binding node.meta?.name ?? freshStructName(fields), so named sets keep their name and only truly anonymous aggregates get structN.
  • Key each sequence field by the child's registered binding name rather than the pre-computed childName. Backends look up structType.fields[binding.name] (collect-field-info, resolve-field-binding), so the key must track the renamed binding. The two are identical for every case except the renamed struct; collapsed children fall back to childName.
  • Guard against the residual clash where a sibling is literally named structN: re-mint whichever colliding party is the auto-named struct so no parameter is silently overwritten (covers both field orderings).

Tests

  • New solver tests: bet-shaped input yields struct1 (not fractional_intensity) with a clean params.struct1.fractional_intensity path; own-field conflict skips struct1 -> struct2; sibling-collision in both orderings keeps both parameters. Shared expectNoRepeatedFieldSegments helper asserts no params.X.X anywhere.
  • Updated two execution.test.ts cases that had encoded the old buggy name (host -> struct1); the joined struct is genuinely anonymous so struct1 is the correct key.

Verification

  • npm test - 1273 passed
  • tsc --noEmit (CI typecheck gate) - clean
  • corpus:roundtrip over the niwrap catalog - 1919/1919 round-trip cleanly
  • typecheck:codegen (generates Python + TS, runs mypy --strict + tsc) - clean

Note

structN is user-visible in generated type/field names. This is inherent: a genuinely anonymous aggregate has no stable semantic name, and this replaces previously-broken colliding names. Deriving a nicer synthetic (e.g. field-name-joined) instead of a bare ordinal is a possible follow-up.

nx10 added 2 commits July 28, 2026 11:03
A multi-field anonymous struct (a `sequence` with no `meta.name`) was named
by `getName`, which via `findDeepName` steals the struct's first field name -
colliding with that field and producing `params.X.X` access paths (e.g.
`params.fractional_intensity.fractional_intensity` for the FSL bet shape).

Add `freshStructName` to `NamingStrategy`: a synthetic `structN` (dedicated
counter) that dodges the struct's own field keys. The genuine-struct and
empty-struct-with-outputs branches use `node.meta?.name ?? freshStructName`,
so named sets keep their name and only anonymous aggregates get `structN`.

Key each sequence field by the child's registered binding name rather than the
pre-computed `childName` - backends look up `structType.fields[binding.name]`,
so the key must track the (possibly renamed) binding. The two are identical for
every case except the renamed struct; collapsed children fall back to childName.
If a synthetic name still clashes with a sibling literally named `structN`,
re-mint the struct so no parameter is silently overwritten.
@nx10
nx10 merged commit a00bd03 into main Jul 28, 2026
3 checks passed
@nx10
nx10 deleted the fix/anonymous-struct-naming branch July 28, 2026 15:17
@nx10 nx10 mentioned this pull request Jul 30, 2026
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