Skip to content

fix: make GenomeEvent use template literal type for proper union narrowing#79

Merged
rsdouglas merged 1 commit intoopenseed-dev:mainfrom
lucamorettibuilds:fix/genome-event-narrowing
Feb 27, 2026
Merged

fix: make GenomeEvent use template literal type for proper union narrowing#79
rsdouglas merged 1 commit intoopenseed-dev:mainfrom
lucamorettibuilds:fix/genome-event-narrowing

Conversation

@lucamorettibuilds
Copy link
Contributor

Fixes #78.

Problem

GenomeEvent = { t: string; type: string; [key: string]: unknown } is a catch-all that absorbs all other members of the Event discriminated union. TypeScript can't narrow event.type === 'creature.boot' because GenomeEvent also matches — forcing as any casts on every property access after narrowing.

Solution

Option A from #78: Make GenomeEvent.type a template literal genome.${string}.

This creates a clean namespace boundary:

  • host.* and creature.* events have literal type strings → TypeScript narrows them perfectly
  • genome.* events are genome-specific and don't overlap

Additional changes

  • Added missing event types that were using as any to emit:
    • HostEvent: budget.exceeded, budget.reset, narrator.entry
    • CreatureLifecycleEvent: creature.spawning, creature.spawned, creature.spawn_failed
  • Added janeeVersion to creature.boot type (was accessed via as any cast)
  • Removed all 5 as any casts from index.ts — they're no longer needed since narrowing works correctly

Migration note

Any genome that emits events with types not prefixed genome. will need to update their event type strings. The host and creature lifecycle events are all accounted for in the updated types.

…owing

Fixes openseed-dev#78.

The catch-all `{ type: string }` in GenomeEvent defeated TypeScript's
discriminated union narrowing on the Event type. This caused all
narrowing checks on event.type to be unreliable, requiring `as any`
casts throughout index.ts.

Changes:
- GenomeEvent.type is now ``genome.${string}`` template literal,
  ensuring it doesn't overlap with host.* or creature.* types
- Added missing event types to HostEvent: budget.exceeded, budget.reset,
  narrator.entry
- Added missing event types to CreatureLifecycleEvent: creature.spawning,
  creature.spawned, creature.spawn_failed
- Added janeeVersion to creature.boot type definition
- Removed all 5 `as any` casts from index.ts that were symptoms of the
  broken union narrowing
Copy link
Contributor

@openseed-patch openseed-patch bot left a comment

Choose a reason for hiding this comment

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

Reviewed the diff carefully and compiled on the branch — clean build.

This is the right fix. The genome.${string} template literal creates a clean namespace boundary that lets TypeScript's discriminated union narrowing actually work. The result: all 5 as any casts gone, and lines 373 and 437 (the ones I flagged in #78 as the remaining hard cases) are fixed too, not just the emit sites.

A few things I checked:

narrator.entry shape — the type definition { text: string; blocks?: Record<string, string>; creatures_mentioned: string[] } matches exactly what narrator.ts emits at line 386–390. No mismatch.

creature.request_restart narrowing — previously (event as any).reason on line 373. Now event.reason works because with GenomeEvent excluded from the creature.* namespace, the guard event.type === 'creature.request_restart' correctly narrows to the CreatureLifecycleEvent variant that has reason: string.

creature.boot narrowing — same fix applies to line 437, event.janeeVersion now resolves cleanly.

Migration note — the PR description correctly calls this out: any genome emitting events without a genome. prefix will break at the type level. That's the right tradeoff. If a genome event type doesn't start with genome., it's using a reserved namespace it shouldn't be touching anyway.

Overlap with PR #77#79 is a strict superset of #77. Recommend closing #77 in favour of this one to avoid a rebase conflict.

One minor observation: narrator.entry is added to HostEvent rather than its own category. That's fine — it's emitted by the host process. But if the narrator grows more event types later, it might be worth a NarratorEvent subtype. Not worth doing now.

Ship it.

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.

GenomeEvent catch-all defeats discriminated union narrowing on Event type

2 participants