Skip to content

feat: parse unknown system-message subtypes into a Generic struct#54

Merged
guess merged 2 commits into
guess:mainfrom
pshoukry:fix/skip-unknown-cli-system-subtypes
Jun 5, 2026
Merged

feat: parse unknown system-message subtypes into a Generic struct#54
guess merged 2 commits into
guess:mainfrom
pshoukry:fix/skip-unknown-cli-system-subtypes

Conversation

@pshoukry

@pshoukry pshoukry commented May 29, 2026

Copy link
Copy Markdown
Contributor

Problem

When a newer Claude Code CLI emits a system-message subtype this SDK version does not model yet (observed: thinking_tokens), the strict subtype dispatch in ClaudeCode.CLI.Parser returns {:error, {:unknown_system_subtype, subtype}}. As a result ClaudeCode.Session.Server drops the message and logs:

[warning] Failed to parse raw message: {:unknown_system_subtype, "thinking_tokens"}

…once per message (500+ occurrences in a single run), and consumers never see the event.

Fix

Add ClaudeCode.Message.SystemMessage.Generic — a fallback struct carrying the raw subtype string plus the full data payload — and return it from the parser for unrecognized system subtypes:

  • The event is delivered with its data instead of being dropped.
  • The SDK stays forward-compatible with new CLI system messages without needing a release (mirrors the Python SDK's generic SystemMessage{subtype, data}).
  • subtype is kept as a string (not an atom), since the set of future subtypes is open-ended (avoids atom-table growth on untrusted-shape input).

When a subtype becomes common enough to warrant typed access, it can be promoted to its own ClaudeCode.Message.SystemMessage.* module and registered in the parser as before.

Tests

  • Unknown subtype (and a thinking_tokens example) parse into Generic with the payload preserved.
  • Full suite green: 1495 tests + 2 doctests, 0 failures.

A newer CLI emits system-message subtypes this SDK version does not model yet
(e.g. thinking_tokens). The strict subtype dispatch returned
{:error, {:unknown_system_subtype, _}}, so ClaudeCode.Session.Server dropped the
message and logged a warning for every one (observed 500+ times in a run).

Add ClaudeCode.Message.SystemMessage.Generic — a fallback carrying the raw
subtype string and the full data payload — and have the parser return it for
unrecognized system subtypes. Consumers now receive the event with its data,
and the SDK stays forward-compatible with new CLI system messages without a
release. The subtype is kept as a string (not an atom) since the set of future
subtypes is open-ended.
@pshoukry
pshoukry force-pushed the fix/skip-unknown-cli-system-subtypes branch from f126e85 to cb2db6b Compare May 29, 2026 04:27
@pshoukry pshoukry changed the title fix: skip unrecognized CLI system subtypes instead of logging parse failures feat: parse unknown system-message subtypes into a Generic struct May 29, 2026
@pshoukry

pshoukry commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@guess can you please check when you get a chance.

@guess

guess commented Jun 4, 2026

Copy link
Copy Markdown
Owner

@pshoukry thx for tagging me, i totally missed this- looking now!

@guess

guess commented Jun 4, 2026

Copy link
Copy Markdown
Owner

Looks good, thanks! Below is claude's review.

Also I'm wondering if Unknown might just be a better name than Generic? I don't feel that strongly about it so will defer to you to consider if you want.

--

Overall: Good fix — returning a fallback struct instead of dropping unknown system subtypes is the right move, and keeping subtype a string for atom-safety is correct. One change needed before merge: the new Generic struct isn't enrolled in the system-message family, so two existing paths don't recognize it.

🔴 Required: register Generic in SystemMessage.type?/1.
ClaudeCode.Message.SystemMessage.type?/1 has no clause for Generic, so it returns false. That has two real consequences I confirmed at runtime:

  • ClaudeCode.Message.message?/1 (message.ex:119) delegates to type?/1, so Message.message?(generic) is false — consumers that filter a stream by message?/1 will drop the very events this PR delivers.
  • Session.Server.extract_session_id/1 (server.ex:473‑475) does if SystemMessage.type?(msg), do: sid — for a %Generic{session_id: ...} the guard is false, so it returns nil and session-id tracking ignores Generic messages that carry one.

Fix is one clause plus the type union:

# lib/claude_code/message/system_message.ex
alias __MODULE__.Generic
# add Generic.t() to @type t
def type?(%Generic{type: :system}), do: true

🟡 Minor:

  • parser.ex:174 — :unknown_system_subtype is now dead in the parse_messages skip set (the parser no longer emits that error). Drop it.
  • No test covers the type?/message?/session-id path for Generic, and there's no generic_test.exs even though every sibling subtype has one. Add a small assertion so a regression in the type?/1 wiring can't ship green.
  • The generic.ex @doc example omits the session_id the code actually sets, and %Generic{subtype:, data:} would violate @enforce_keys [:type, :subtype] if it were ever wired as a real doctest. Worth correcting.

@pshoukry

pshoukry commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@guess thanks will fix accordingly.

I prefer generic since ( we have a subtype returned like thinking_tokens ) but we are not handling it a specific handling. ie: We are handling it generically until we do a specific implementation / handling. Since you aren't feeling strongly about it I will keep it as is for now!

- Register Generic in SystemMessage.type?/1 and the @type union so
  Message.message?/1 and Session.Server session-id tracking recognize it
  (previously Generic events were delivered by the parser but dropped by
  message?/1 stream filters and ignored for session-id extraction).
- Drop dead :unknown_system_subtype from the parser skip set; the parser
  no longer emits that error.
- Correct the Generic.new/1 @doc example to reflect the fields the code
  sets (type, session_id) so it satisfies @enforce_keys.
- Add generic_test.exs covering new/1, generic?/1, and the
  type?/message? family-enrollment wiring, plus run the doctest.
@pshoukry

pshoukry commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 1186162:

  • 🔴 Registered Generic in SystemMessage.type?/1 and added Generic.t() to the @type t union. Message.message?/1 and Session.Server session-id extraction now recognize Generic messages, so stream filters no longer drop them and a session_id they carry is tracked.
  • 🟡 Removed the now-dead :unknown_system_subtype from the parse_messages skip set.
  • 🟡 Added generic_test.exs covering new/1, generic?/1, and the type?/message? family-enrollment wiring, and wired the module's doctest so the type?/1 wiring can't regress green.
  • 🟡 Corrected the Generic.new/1 @doc example to include the type and session_id the code sets (now a runnable doctest that satisfies @enforce_keys).

On naming: keeping Generic — the subtype is known (e.g. thinking_tokens), it's the handling that's generic until a dedicated struct is added.

Full suite green (the 2 transient failures I saw locally were MockCLI readiness timeouts under parallelism; both pass in isolation).

@guess
guess merged commit 3fb3b3d into guess:main Jun 5, 2026
2 checks passed
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.

2 participants