Python SDK 3.1.2: lazy imports, attribution header, SSE forward-compat, answer safesearch

[tyler5673]

Superseded by #49.

The branch was squashed to a single commit and reopened as a fresh draft so
review could start from a clean slate. See #49 for the current description,
which also reflects a later change to the attribution header's source token.

[factory-droid]

Droid finished @tyler5673's task —— View job

---

Droid review (Phase 2 validation): One concrete fix needed, normalize recorded header keys in the new live attribution test to avoid a casing-related KeyError; otherwise the changes look cohesive.

[factory-droid]

[P1] Live attribution test can KeyError on header casing

record() stores dict(request.headers) with whatever casing httpx uses (for example X-Client-Info). The test checks for the header case-insensitively but then indexes h["x-client-info"], which can raise KeyError and make the live test flaky.

Suggested change

	observed.append(dict(request.headers))
	observed.append({k.lower(): v for k, v in request.headers.items()})

[tyler5673]

Fixed in 3fe1dca. Confirmed the failure mode is latent rather than live — dict(httpx.Headers) lowercases today, so the index worked, but the surrounding guard was case-insensitive while the lookup was not, which is self-contradictory and would have turned any future casing change into a KeyError instead of a clean assertion failure.

Took the suggested normalization at record time, and then simplified what it made redundant: the match comprehension is now a plain "x-client-info" in h membership test instead of a case-insensitive check followed by a case-sensitive index, and the MCP-absence check drops its per-name .lower(). Both live tests still pass against prod.

[factory-droid]

Droid finished @tyler5673's task —— View job

---

Droid review (Phase 2 validation): The changes look cohesive and well-tested for the new lazy root surface, attribution header, and SSE forward-compat behavior. The main remaining gap is that the new Event/EventName docs examples are not copy-paste runnable and should be made self-contained per the repo’s documentation contract.

[factory-droid]

[P2] Make open-enum example runnable

The “Open enum” snippet references evt without defining it, so copy-pasting fails with NameError; please make the example self-contained (for example, validate a ResearchTaskStreamEvent with a known and an unknown event name) to match the repo’s docs contract that examples are runnable.

[tyler5673]

Fixed in 6dff3f4. Both snippets are now self-contained, and I ran every ```python block on both pages rather than eyeballing them — 6/6 execute clean, and the # output comments in the docs are the real captured stdout.

event.md builds a known and an unknown ResearchTaskStreamEvent via model_validate before the isinstance guard; eventname.md imports Union/Event for the alias and constructs a two-frame stream list so the loop has something real to iterate. Good catch — AGENTS.md "Examples must be copy-paste runnable" applies to docs/**/*.md, and the pre-existing Event.CONNECTED example on that page was already honouring it, so mine were the outliers.

[factory-droid]

[P2] Make EventName docs example runnable

This page shows Union without importing it and iterates stream without defining it, so copy-paste fails; please include the needed imports and construct a minimal local stream (or show a direct ResearchTaskStreamEvent.model_validate(...) example) so readers can run it as-is.

[tyler5673]

Fixed in 6dff3f4. Both snippets are now self-contained, and I ran every ```python block on both pages rather than eyeballing them — 6/6 execute clean, and the # output comments in the docs are the real captured stdout.

event.md builds a known and an unknown ResearchTaskStreamEvent via model_validate before the isinstance guard; eventname.md imports Union/Event for the alias and constructs a two-frame stream list so the loop has something real to iterate. Good catch — AGENTS.md "Examples must be copy-paste runnable" applies to docs/**/*.md, and the pre-existing Event.CONNECTED example on that page was already honouring it, so mine were the outliers.

[factory-droid]

Droid finished @tyler5673's task —— View job

---

Droid review (Phase 2 validation): Changes look cohesive and well-covered by tests. One doc fix remains, the “Note on streaming” section in MIGRATION.md still reflects the old strict-enum behavior for stream_research_task() and should be updated for the new open-enum SSE behavior.

[factory-droid]

[P1] Update MIGRATION streaming guidance for open-enum SSE

MIGRATION.md now states unknown SSE event names no longer raise ResponseValidationError as of 3.1.2, but the later “Note on streaming” still claims you.stream_research_task() validates against a fixed Event enum and will raise on intermediate events; please update or qualify that note so the docs match the new open-enum behavior and do not steer users away from stream_research_task for a problem that is now fixed.

[tyler5673]

Addressed the streaming-note finding in 9cd40f2, and the sweep turned up a second instance the review didn't flag.

Verified the claim first. MIGRATION.md asserted stream_research_task() raises ResponseValidationError on the first intermediate event, naming response.created, response.starting, response.output_item.added. I streamed exactly those three through the method over a MockTransport: all decode as plain str, no exception. Stale as of this release, confirmed rather than assumed.

MIGRATION.md — the note lives in the historical 2.4.0 → 2.5.0 section, so I did not rewrite it. It is now labelled "as written for 2.5.0" with an "Updated in 3.1.2" block beneath, preserving the record while stopping it misleading a current reader.

README.md (not flagged, same defect, higher blast radius) — line 251 said to prefer stream_research() over stream_research_task() because the latter "validates strictly and will raise on an unrecognized event." Equally false now, and in the user-facing README rather than a historical migration section.

The recommendation survives, the reason changes. stream_research() is still preferable for long tasks, because stream_research_task() retries [429, 500, 502, 503, 504] on stream-open and can silently reopen a half-consumed stream, while stream_research() deliberately skips those retries — confirmed against sdk.py:500 and the _open_raw_stream comment in research_helpers.py. It also tolerates non-JSON data frames, which the pydantic path does not. So its remaining edge is shape tolerance, not name tolerance, and I corrected the two research_helpers docstrings that still called the pydantic decoder "strict".

Gates after the change: mypy clean, pylint --enable=E exit 0, research_helpers.py 10.00/10, 285 tests passing, no OpenAPI drift.

[factory-droid]

Droid finished @tyler5673's task —— View job

---

Droid review (Phase 2 validation): No candidate inline comments to post. After reading the full PR diff, I did not find any additional high-confidence, actionable issues, and there are no security findings in this pass.

[factory-droid]

Droid encountered an error —— View job

---

Droid is reviewing code and running a security check…

[factory-droid]

Droid encountered an error —— View job

---

Droid is reviewing code and running a security check…

[factory-droid]

Droid finished @tyler5673's task —— View job

---

Droid review (Phase 2 validation): Validated all candidates against the full diff. Posted 3 inline comments for small but real contract mismatches (empty app_name + app_version silently dropping on the wire, USAGE attribution docs missing the / constraint, and attribution module docstring drift).

[tyler5673]

Superseded by a fresh draft PR. The branch is squashed to a single commit (99ef0c9) covering the review fixes plus the attribution-shape changes agreed after inspecting the MCP server's real buildClientInfoHeader: source token sdk, caller-supplied client= dropped when unset, and keyword-only attribution args. Reopening as a new PR to retrigger review from a clean slate rather than layering onto resolved threads.

[factory-droid]

[P1] Reject app_version when app_name is empty

You.__init__ only raises when app_name is None, but build_client_info_header() only emits the client= segment when app_name is truthy, so You(app_name="", app_version="1.2.3") passes validation and then silently drops the version on the wire. Treat an empty app_name as unset in the guard (and align the validation condition) so this misconfiguration fails fast instead of producing an incorrect header.

Suggested change

	if app_version is not None and app_name is None:
	raise ValueError(
	"app_version requires app_name; the attribution header emits "
	"them together as client=<name>/<version>, so a version with "
	"no name has nowhere to go."
	)
	if app_name is not None:
	validate_attribution_arg("app_name", app_name, forbidden=";/")
	if app_version is not None:
	validate_attribution_arg("app_version", app_version, forbidden=";/")
	if app_title is not None:
	validate_attribution_arg("app_title", app_title)
	if app_url is not None:
	validate_attribution_arg("app_url", app_url)
	if app_version is not None and not app_name:
	raise ValueError(
	"app_version requires a non-empty app_name; the attribution header emits "
	"them together as client=<name>/<version>, so a version with "
	"no name has nowhere to go."
	)
	if app_name:
	validate_attribution_arg("app_name", app_name, forbidden=";/")
	if app_version is not None:
	validate_attribution_arg("app_version", app_version, forbidden=";/")
	if app_title is not None:
	validate_attribution_arg("app_title", app_title)
	if app_url is not None:
	validate_attribution_arg("app_url", app_url)

[factory-droid]

[P1] Document the / constraint for app_name and app_version

The attribution example says values must exclude only ;, but the implementation also rejects / in app_name and app_version (since analytics parses client=<name>/<version> on /), so this block is currently out of sync with the real validation contract.

Suggested change

	`app_name`, `app_version`, `app_title` and `app_url` are all optional. When
	omitted, those segments are dropped entirely. Values must be printable ASCII (excluding `;`); invalid
	values raise `ValueError` at construction time.
	`app_name`, `app_version`, `app_title` and `app_url` are all optional. When
	omitted, those segments are dropped entirely. Values must be printable ASCII (excluding `;`), and
	`app_name` / `app_version` also exclude `/`; invalid values raise `ValueError` at construction time.

[factory-droid]

[P2] Fix attribution module docstring drift

The module docstring says build_client_info_header lazily imports both httpx and youdotcom “for the version pin”, but the implementation only imports httpx and does not inspect youdotcom at all, so the rationale is misleading in a PR that is explicitly about keeping import youdotcom transport-free.

Suggested change

	"""Build the ``X-Client-Info`` header value for outbound SDK requests.
	Emits a caller-identity header so the analytics layer can distinguish
	SDK traffic from other sources. SDK traffic is uniquely identified by
	the leading literal ``sdk``.
	``build_client_info_header`` is called per-request from
	``BaseSDK._build_request_with_client`` immediately after the
	``User-Agent`` header is set. It does no module-level transport
	imports — ``httpx`` is pulled in lazily at the top of the function
	body (as is ``youdotcom`` itself, for the version pin) so that
	``import youdotcom`` does not regress because of this module.
	"""
	"""Build the ``X-Client-Info`` header value for outbound SDK requests.
	Emits a caller-identity header so the analytics layer can distinguish
	SDK traffic from other sources. SDK traffic is uniquely identified by
	the leading literal ``sdk``.
	``build_client_info_header`` is called per-request from
	``BaseSDK._build_request_with_client`` immediately after the
	``User-Agent`` header is set. It does no module-level transport
	imports — ``httpx`` is pulled in lazily at the top of the function
	body so that ``import youdotcom`` does not regress because of this
	module.
	"""