docs: correct the Python and Go event data model tip - #2160
Merged
Conversation
The Python/Go tip in the streaming events reference described a single `Data` class/struct with every field optional, where fields that are not populated for an event come back as `None`/`nil`. Both bindings use a separate data type per event, so code written against that tip does not work: in Go `event.Data` is the `SessionEventData` interface and reading a field off it does not compile, and in Python `event.data` is a per-event dataclass, so reading a field that belongs to another event raises `AttributeError` instead of returning `None`. The tip was accurate when it was added and went stale when both bindings moved to per-event data types. Describe the per-event model instead, using the wording the same file already uses for .NET, and correct the copy of the same sentence that the docs style guide uses as its example of a language-qualified callout.
Contributor
There was a problem hiding this comment.
Pull request overview
Corrects Python and Go streaming-event documentation to reflect their per-event payload types.
Changes:
- Updates the streaming-events tip.
- Aligns the docs style-guide example.
Show a summary per file
| File | Description |
|---|---|
docs/features/streaming-events.md |
Corrects the Python and Go event data model guidance. |
.github/instructions/docs-style.instructions.md |
Updates the language-qualified callout example. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Medium
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
docs/features/streaming-events.mdtold Python and Go users that session event payloads use a singleDataclass/struct with every field optional, and that fields not populated for an event come back asNone/nil. Both bindings use a separate data type per event, so neither behaves that way.Fixes #2159
Why the old text was wrong
Go.
event.Datais theSessionEventDatainterface (go/rpc/zsession_events.go), which carries no data fields, so reading one off it does not compile:toolNameis listed in the doc's owntool.execution_starttable, so this is a field the tip said was readable for that event.Python.
event.datais a per-event dataclass, so reading a field that belongs to a different event raises instead of returningNone:The class named
Datastill exists in the Python binding, but it is a shim for manually constructed payloads with no declared fields; event delivery never produces it, and unrecognized events fall back toRawSessionEventData.The tip was accurate when it was written. Go moved to per-event data structs in #1037 and Python in #1063. The Go change updated this file's Go example and left the prose behind. Docs validation extracts and compiles fenced code blocks only, so nothing checks prose, which is why this file's own Go example uses
event.Data.(*copilot.AssistantMessageDeltaData)about sixty lines above a tip saying that is unnecessary.What changed
docs/features/streaming-events.md- the Python/Go tip now describes per-event data types, using the wording the file already uses for the .NET tip directly below it..github/instructions/docs-style.instructions.md- the same sentence is used there as the example of a language-qualified callout, so it carried the same incorrect claim.Prose only. No code, generated files, codegen templates, schemas, samples or tests.
Verification
npm run validate:go(48/48) andnpm run validate:py(57/57) pass, and the extracted code corpus is byte-identical tomainsince only prose changed.Not included
Java uses per-event classes and Rust exposes the payload as untyped JSON. Neither has a tip in this section today and this change does not add one.