Conversation
Adds support for the new `knowledge` request parameter on POST /v1/search and the knowledge result models that come back with it. Request: `knowledge="core"` on `search()` / `search_async()` and the deprecated `you.search.unified*` shims. Normalized to lowercase like the other enum-typed params; an invalid value raises ValidationError locally before any network call, mirroring the server's 422. All params on these methods are keyword-only, so the addition is non-breaking. Response: `Results.knowledge` is an optional list of `KnowledgeResult`, each carrying `type`, `title`, and `attribution`, plus `description` and an optional `as_of` date for `type: answer` results. `type` is a plain `str` so an unrecognized future kind parses instead of raising. The API omits the key entirely when nothing relevant is found, so the field is None rather than an empty list. Also fixes a blind spot in scripts/check_drift.py: the response-schema check compared top-level fields only, so a new nested field such as `results.knowledge` was invisible. It now recurses through nested object schemas and arrays, and falls back to the previous flat comparison when a response resolves to a oneOf union rather than a single object schema. Recursion surfaced two pre-existing gaps unrelated to this change (AnswerSearchResult.description/.thumbnail_url and FinanceResearchSource.snippets); those are listed in an explicit KNOWN_RESPONSE_GAPS table that reports itself stale once the SDK catches up. Verified: 425 offline tests pass, 6 live knowledge tests pass against prod, mypy clean over 85 files, pylint 10.00/10 on the CI gate, and the drift check is clean in both --verbose and --strict modes.
… (DX-835) Two of three review findings verified valid and fixed here. check_drift.py: the KNOWN_RESPONSE_GAPS staleness check compared against `missing_in_sdk`, so a field the spec *dropped* was reported as stale with the message "the SDK model now defines it" -- false, and it would fail a strict drift run for the wrong reason. Staleness now compares against the model's own fields (`known & model_fields`). Negative-tested four ways: spec-drops-field emits nothing, SDK-defines-field still reports stale, genuine new drift is still caught, and existing suppressions still hide the known gaps. The previous logic reproduced the false positive on the first case. test_live.py: test_knowledge_result_shape hard-asserted `type == "answer"` and required `description` on every result, contradicting the forward-compat design behind modeling `type` as a plain str. The spec states description is "Required on `type: answer` results" only, and that an unrecognized kind should be ignored rather than failed on. The test now asserts the fields the spec requires of every kind and scopes description to `type == "answer"`. Rejected: rewording the Knowledge docstring to drop "alongside web and news search". That sentence is verbatim from the published OpenAPI spec, which the model docstrings mirror by convention. The live API contradicting it is the server-side bug tracked separately; rewriting the SDK contract to match a launch-blocking bug would desync the SDK from the spec and docs.
The Knowledge results snippet used a bare `you`, so pasting it alone raised NameError, and it made a network call without timeout_ms. Its direct structural precedent -- `#### Page content extraction`, the same kind of new-search-param subsection at the same heading level -- is fully wrapped with imports, client construction, and timeout_ms=60_000. The bare-style snippets in this README are only the short top-level API-overview blurbs that follow the Quickstart. Now matches the extraction subsection. Verified by extracting the rendered snippet and running it verbatim against prod: prints knowledge card titles, descriptions, and attribution credits.
`Results.web` and `Results.news` are `Optional[List[...]]` in
`searchresponse.py`, but their `docs/models/results.md` type cells read
plain `List[...]`. The wrong cells pre-date this branch, but adding the
`knowledge` row re-padded the table and re-emitted both lines, so they
are changed lines in this diff.
AGENTS.md ("Type cells in field tables come from the annotation") names
`Optional[X]` vs `X` as a trap and requires eyeballing every row of a
touched model page, not just the rows whose prose changed.
Verified with an annotation-vs-docs comparison across all four touched
model pages (25 rows): all match, including the new `knowledge` row.
…ert (DX-835) Two findings from the second droid-review round, both verified valid. Cross-table identity: the `knowledge` description in docs/sdks/you/README.md, docs/sdks/search/README.md and docs/models/searchrequestbody.md carried the `"core"`-only / ValidationError / 25-result / `count` constraints, but both `SearchRequestBody` field docstrings stopped after the first sentence. AGENTS.md requires those surfaces to read identically. Expanded the docstrings rather than shrinking the tables, matching how `include_domains` and `extraction` already carry SDK-layer guidance past the spec sentence. The `Knowledge` enum class docstring stays spec-verbatim: it is not one of the four cross-table surfaces, `components.schemas.Knowledge.description` is exactly that sentence (re-fetched), and `SearchRequestBody.properties.knowledge` carries no description of its own. `FreshnessValue` and `LiveCrawlFormats` likewise have enum docstrings that differ from their field docstrings. Live assert: `assert kr.description` -> `assert kr.description is not None`. `description` is `Optional[str]`, so `is not None` is the real presence check; truthiness would also enforce non-empty, which AGENTS.md's "`is not None`, not truthiness" rule warns against for server-returned strings. `type`, `title` and `attribution[].name` keep truthiness deliberately: those are required `str` fields, where `is not None` is vacuous (pydantic already guarantees it) and non-empty is the meaningful claim.
…ntory (DX-835) Docs-only. Two independent staleness fixes found while auditing every doc surface in the repo. Type cells: 11 rows across 8 model pages declared a bare type where the annotation is Optional -- contentsrequest (urls, formats), sourcecontrol (include_domains, exclude_domains, boost_domains), source (snippets), searchrequest (livecrawl_formats), researchdetail and financeresearchdetail (ctx), researchrequest (output_schema), researchtaskstreameventdata (data). AGENTS.md names Optional[X] <-> X as a type-cell trap. All pre-date this branch. Every cell fits its existing padded column, so only those 11 lines change and table alignment is preserved. The Required cells were already right: is_required() is False for all 11. tests/README.md: listed 11 of the 20 test files and carried wrong counts (Contents 12 vs 13, Answer 23 vs 25). Now lists all 20, adds groups for extraction, knowledge, page_age, stream events and the cross-cutting suites, and labels counts as pytest --collect-only numbers. The groups sum to 425, exactly the CI-gate collection. Verified with a heading-aware audit comparing every field table in docs/models/ against typing.get_type_hints -- 151 rows, all four AGENTS.md traps (Optional<->X, datetime<->date, bool<->int, str<->int): 0 findings left. The 15 enum "## Values" pages were also diffed against their code members: clean, the one apparent hit being contentsformats' intentional "metadata _(deprecated)_" annotation.
…o the retries example (DX-835) Two docs footguns, one from review and one found sweeping every snippet. docs/models/knowledgeresult.md: the `as_of` note told readers to call `datetime.strptime(card.as_of, "%Y-%m-%d")` without an import, so following it raised NameError. Replaced with a fenced block that carries `from datetime import datetime`, matching the pattern docs/models/webresult.md already uses for its own `page_age` datetime note. The block also guards the optional section (`res.results.knowledge or []`) and uses `is not None` rather than truthiness, consistent with the model's Optional[str] annotation. README.md: the Retries example builds its own client but passed no `timeout_ms`, so copying it inherits httpx's 5s default and raises ReadTimeout -- the exact failure the adjacent Timeouts section warns about. AGENTS.md requires an explicit `timeout_ms` on any snippet that triggers a round-trip. The other README snippets that call `you.search` / `you.contents` without a `timeout_ms` are deliberately untouched: they reuse the client constructed in Quick Start, which already passes `timeout_ms=60_000`, so the rule is satisfied once upstream rather than per snippet.
Two spots in tests/test_knowledge.py referenced a `knowledge` value that the published spec does not define -- one in a docstring that also described it as not public yet, which discloses an unreleased parameter value in a public repo, and one as the input to the invalid-value case. Both now state the contract without naming anything unpublished. The docstring explains that `core` is the only value the published spec defines and why the enum is pinned to exactly that, and the invalid-value case uses an obviously non-value string instead. Coverage is unchanged: the enum is still pinned to ["core"], and an unrecognized value still raises ValidationError locally before any request is sent. 21 knowledge tests and the 425-test offline gate pass, mypy clean, pylint 10.00/10 on the CI errors-only gate, drift --strict no_drift.
…ess (DX-835) The two async knowledge tests each re-declared the mock-transport handler and the AsyncClient/You scaffolding already living in `_capture`, differing only in the response body. Extracted `_acapture` as the async twin. Worth doing for the lifecycle rather than the line count (net +1): the suite treats a leaked transport as a failure via ResourceWarning-as-error, and the copy-pasted form put an `await client.aclose()` in every caller's hands. One helper now owns it, so a future async test cannot forget it. Coverage unchanged: 21 knowledge tests and the 425-test offline gate pass with no ResourceWarning.
…he (DX-835) _compare_response_fields() used `visited` as an "already compared this model" cache, returning early whenever the same pydantic class reappeared. One model can legitimately sit at several response paths backed by different spec schemas, so every branch after the first was skipped and real drift went unreported. Reproduced before fixing: a root model with two fields of the same type whose spec schemas differ by one field reported drift on the first path only and silently missed the second. `visited` is now discarded on exit via try/finally, so it breaks cycles without suppressing sibling branches. Checked against self-referential and mutual A->B->A schemas: both still terminate, and drift inside the cycle is still reported. Fixing the cache unmasked one genuine asymmetry, now suppressed explicitly rather than left as a standing warning: web-search results.news.contents: SDK has `highlights`, spec does not results.web[].contents resolves to WebContentsPost, which defines `highlights`, while results.news[].contents resolves to the narrower Contents schema (html, markdown). The SDK shares one Contents model for both. Confirmed against prod that highlights never arrives on the news path: with extraction_mode=highlights news items carry no contents at all, and with the deprecated livecrawl=all they carry html only. Narrowing the model would be a breaking change for no behavioral gain, so KNOWN_SHARED_MODEL_EXTRAS records the gap. It mirrors KNOWN_RESPONSE_GAPS and is self-invalidating the same way, reporting the entry as stale if the spec ever defines the field at that path. drift --verbose and --strict both clean. 425 offline tests, all 46 live tests against prod, mypy clean over 85 files, pylint 10.00/10 on the CI gate.
…tale descriptions (DX-835) Five review findings: four fixed, one rejected. `SearchResponse.results` is `Optional[Results]` and not required, so `for card in res.results.knowledge or []:` still raises AttributeError when a response omits `results` -- the `or []` only guards the inner key. Guarded all five snippets (README, USAGE, and three in docs/models/knowledgeresult.md) with `if res.results:`, matching examples/api-example-calls.py, which already did it correctly. `SearchResponseTypedDict` / `SearchResponse` described the payload as "unified search results from web and news sources"; `Results` now carries a third section. Updated both docstrings and the mirrored docs/models/searchresponse.md. That text is SDK-authored -- `SearchPostResponse.description` is null in the published spec -- so there is no spec wording to stay verbatim against. The endpoint summaries in docs/sdks/* and sdk.py are left alone deliberately: they quote the spec's operation.summary verbatim, the spec has not been updated, and knowledge is opt-in, so describing the endpoint's default output as web and news is still accurate. The knowledge perf case measures the request side only under the default mock target, because tests/mockserver returns one fixed payload regardless of the request body and includes no knowledge section. Documented that in the docstring rather than changing the Go mock: the mock ignores the request entirely, so emitting knowledge conditionally would mean parsing the body, and the pre-existing extraction cases have the same property. Rejected: the claim that tests/README.md undercounts test_performance.py (33 vs 34). pytest collects 33; the 34th `def test_` line is `test_target`, a module-level @pytest.fixture, not a test. The doc's counts are labelled as `pytest --collect-only` numbers and match.
…esearch (DX-835) Two response models were narrower than the spec they parse against, so documented fields were discarded at parse time. AnswerSearchResult lacked `description` and `thumbnail_url`. The answer spec defines both on results.web[] items and prod returns them: a live call came back with description on 20/20 web results and thumbnail_url on 16/20, all silently dropped. WebResult on the search endpoint already declared both, so the answer model was simply the narrower of two siblings describing the same shape. FinanceResearchSource lacked `snippets`. The finance-research spec defines it on output.sources[] and the sibling Source model on the Research API already declared it, with an identical description. Prod was not returning it -- two live calls at `deep` and `exhaustive`, 10 sources between them, all carrying only title and url -- so nothing was being lost yet. Added anyway: it was held back by a KNOWN_RESPONSE_GAPS suppression whose stale check can only see the SDK catching up, never the API starting to honour its own spec, so that gap would have stayed silent from the API side indefinitely. Both suppressions are removed and KNOWN_RESPONSE_GAPS is now empty. drift --verbose reports no drift with an empty table, which is the actual proof the gaps closed rather than got hidden; the stale mechanism was negative-tested separately so it is still live for the next one. The Go mock for finance_research now emits `snippets`, and its comment no longer claims the model defines only url and title. tests/test_research.py asserted that old behaviour and now asserts snippets parse. tests/test_answer.py feeds both new fields through _ANSWER_BODY and pins them present on the first web result and None on the second, covering optionality as well. Folds into the unreleased 3.5.0 rather than bumping again; CHANGELOG records both under Fixed.
…at lacked it (DX-835) `SearchResponse.results` is `Optional[Results]`, so `assert res.results.knowledge` raises AttributeError on a response that omits `results`, hiding the assertion that was actually meant to fail. Three of the six tests in TestLiveSearchKnowledge already asserted `res.results is not None` first; the other three did not. All six now do. Verified by parsing the class and confirming, per test, that the guard precedes the first dereference rather than just that both strings appear. 6 live knowledge tests pass against prod; 425 offline tests unchanged.
…X-835)
Three of the four TestLiveContents cases could pass without the requested field
ever arriving:
test_html_format guarded with `if res[0].html:`, so a response carrying no
html asserted nothing -- while its own comment said
"HTML should be present when HTML format is requested"
test_markdown_format asserted only that a non-empty list came back
test_multiple_formats same; never checked html or markdown at all
test_metadata_format in the same class already did this correctly
(`assert res[0].metadata is not None`), so the other three now match it.
Presence is asserted with `is not None` rather than truthiness, per AGENTS.md's
rule for server-returned strings.
Pre-existing, and untouched by this branch until now. Verified against prod
after strengthening rather than assuming: all 5 Contents live tests pass, so the
fields really do arrive and the assertions were simply missing. The full
non-research live suite (31 tests) and the 425-test offline gate pass.
…-835) The `knowledge` parameter description opened with "Requests knowledge results alongside web and news search", which reads as a promise that those sections accompany knowledge results. `SearchResponse.results` and each of its sections are optional, and this PR's own description records that prod currently drops `results.web` entirely when knowledge is requested. The docs and tests here deliberately make no claim about `results.web` alongside knowledge, so the parameter wording was the one surface still implying it. Reworded the opening to "Requests knowledge results from licensed data providers." -- accurate, matches how the CHANGELOG and USAGE already describe knowledge, and makes no claim about other sections. It is one character shorter than the sentence it replaces, so all three parameter tables keep their existing row padding; a longer rewrite would have forced re-padding every row of three tables for a wording change. Applied to the five SDK-authored surfaces the cross-table identity rule covers (both SearchRequestBody field docstrings, docs/sdks/search/README.md, docs/sdks/you/README.md, docs/models/searchrequestbody.md) and verified identical afterwards. SearchRequestBody.properties.knowledge carries no description in the published spec, so this text is entirely SDK-authored. Deliberately NOT applied to the two spec-verbatim surfaces: the Knowledge enum class docstring and docs/models/knowledge.md mirror components.schemas.Knowledge.description, which is exactly "Requests knowledge results alongside web and news search." AGENTS.md pins model docstrings to the spec, so those follow the spec rather than editorialising ahead of it; if the upstream wording changes they change with it.
… page (DX-835)
Two review findings.
check_drift.py: `stale = known & model_fields` contradicted the comment sitting
directly above it. A field the spec *dropped* is still in model_fields, so its
suppression was reported stale even though the spec no longer defines it -- while
the comment said that case "must not be reported as stale". Intersecting with
`props` makes the code match the stated intent. Proven real rather than
theoretical: with spec props {kept}, suppressed {added} and SDK fields
{kept, added}, the old expression yields stale={added} and the new one yields
empty. All three staleness cases now verified -- gap still open (not stale), SDK
caught up (stale), spec dropped the field (not stale).
The mirror table KNOWN_SHARED_MODEL_EXTRAS already intersected with props, so it
was correct; the two directions differ and only this one was wrong.
docs/models/knowledge.md: the H1 said knowledge is requested "alongside web and
news search" -- the same composition promise removed from the parameter surfaces
in 34bde1e. I declined to change this page then, on the grounds that it mirrors
the spec-verbatim Knowledge enum docstring. That was over-broad. AGENTS.md's
spec-verbatim rule binds model docstrings; docs pages carry their own prose.
docs/models/extraction.md, the page AGENTS.md names as the format template, does
not match the Extraction class docstring, and contentsmetadata.md uses a richer
deprecation callout. Across all 65 model pages, 22 mirror their class docstring
exactly once RST/markdown backtick dialect is normalised and 7 genuinely carry
their own wording. So the page is reworded and the enum docstring stays verbatim.
Worth noting: scripts/check_drift.py has no committed test coverage at all, which
is why two bugs in this one function surfaced only through review.
scripts/check_drift.py had no committed test coverage, which is why two bugs in one function -- the `visited` global cache and the staleness intersection -- surfaced through review rather than through a failing test, and why each reproduction written while fixing them was thrown away afterwards. tests/test_check_drift.py loads the script by path (it is not a package module) and pins 22 cases: response recursion down through arrays and $refs, drift on a sibling branch that reuses a model, cycle safety for self-referential and mutual A->B->A schemas, oneOf bail-out, all three staleness cases for both suppression tables, the _resolve_schema and _nested_model helpers, and the shipped state of both tables. Both regressions were verified to actually fail when their fix is reverted and pass when it is restored, so these are regression tests rather than coverage for its own sake. The script is byte-identical afterwards. Offline gate 425 -> 447. tests/README.md inventory and group counts updated to match: 21 files listed, groups summing to exactly the CI-gate collection.
…ten two weak live tests (DX-835)
check_drift.py compares the published specs against the SDK models. That cannot
see a field the API returns but no spec declares -- which is exactly how
AnswerSearchResult came to drop `description` and `thumbnail_url` while every
static check agreed with itself and reported no drift.
scripts/audit_wire.py closes that gap: it walks the raw JSON from a live call
next to the parsed model and reports any key the model discarded. Across all
seven endpoints that is 228 wire keys, none unexplained. It needs YDC_API_KEY so
it is a pre-release check, not a CI gate; --strict exits 1 on an unexplained
drop, negative-tested by emptying the known list.
Two keys are recorded in KNOWN_WIRE_EXTRAS as observed-but-undeclared rather than
modeled, so the tool stays quiet about decisions instead of hiding accidents:
results.web[].original_thumbnail_url declared by no published spec
finance-research root `warnings` declared by the sibling research spec
and sent by prod as [], but absent from
finance-research.json
`warnings` is the interesting one. Adding it to FinanceResearchResponse is
tempting -- ResearchResponse has it and prod sends it -- but finance-research.json
declares only `output`, so the field would put the SDK permanently ahead of that
endpoint's contract and leave a standing drift warning. Modeled it, watched
check_drift --strict fail on exactly that, and reverted. The rule that falls out
is the one already applied to snippets: add the field when the endpoint's spec
declares it, record it when only prod does. A test pins the current state so
closing the gap later is a deliberate act.
Also here:
- docs/models/researchresponse.md never documented `warnings`, though the field
has always existed and parsed. Found by a missing-row audit; the type-cell
audit could not see it because it only validates rows that already exist.
- tests/test_live.py TestLiveSearch.test_search_with_filters guarded its only
substantive assertion behind `if res.results.web:`, so an empty response
passed and the filters went unverified despite the name. Now asserts the
section is populated; verified against prod over five consecutive runs.
- two pre-existing mypy errors in check_drift.py, from indexing a heterogeneous
list of dicts, so `mypy src/youdotcom/ scripts/` is clean at 87 files.
Offline gate 425 -> 450. tests/README.md counts updated to match.
…ransport (DX-835) Two review findings. sdk.py's `_search_impl` `:param knowledge:` still said knowledge is returned "alongside web and news search" -- the same composition promise removed from the five parameter surfaces in 34bde1e. It survived because that sweep grepped line by line and this copy wraps across two lines. A whitespace-normalised search over src/, docs/, README, USAGE and CHANGELOG now finds the phrase in exactly one place: the Knowledge enum docstring, which stays verbatim against components.schemas.Knowledge.description. scripts/audit_wire.py's _Spy wrapped an httpx.HTTPTransport but never closed it. BaseTransport.close() is a no-op and Client.close() only calls the outer transport, so the inner pool leaked once per audited call -- seven times on a full run. Added close() delegating inward, and verified both directions: with the override the inner transport's close() is called exactly once, and a control spy without it is called zero times.
… pattern (DX-835) docs/models/results.md was a bare field table with no prose, unlike its sibling searchresponse.md. That matters here: all three sections are Optional and independent, `results` itself is Optional, and prod currently omits `web` entirely whenever knowledge is requested -- returning a completely empty results object for roughly half the queries tested. A user reading the field table alone had nothing telling them to guard each level. Added the contract statement and a runnable example. The example was executed verbatim against prod on a query that returns no `web` section: it parses, meets the copy-paste-runnable and timeout_ms rules, and runs clean. The wording describes optionality, which the schema guarantees and which stays true however the server-side issue resolves. It deliberately does not say "`web` is omitted when knowledge is requested" -- that would document a bug as contract, be wrong once fixed, and nothing would catch the staleness, because check_drift.py compares field names, enums, endpoints and servers but never descriptions.
The help said "list kept keys too", but the flag never printed kept keys -- both branches print dropped keys and differ only in path normalization. Reworded to describe the real behavior and why the default is the more useful one: the normalized form (results.web[].x) is what matches KNOWN_WIRE_EXTRAS, while --verbose preserves list indices (results.web[0].x) for locating a specific item. Verified against a run with the known-extras list emptied, so the dropped-key paths are actually printed in both modes: default normalizes, --verbose does not.
…corded (DX-835) `livecrawl=web` combined with `livecrawl_formats=[markdown]` no longer returns `contents` on any web result. Verified against prod: livecrawl=web + formats=[markdown] 0/3 web results have contents <- fails livecrawl=all + formats=[markdown] 2/3 livecrawl=web (no formats) 2/3 extraction full_page markdown 3/3 extraction highlights 3/3 So it is one deprecated combination, and the replacement works fully. The test fails identically on origin/main in a clean worktree, so it predates this branch and is not a regression from the knowledge work. Marked xfail rather than relaxed or deleted: relaxing the assertion would bake in behavior nobody has said is intended, and deleting it would lose the record. The reason string carries the measured matrix so the next reader does not have to re-derive it. Non-strict, so it xpasses harmlessly if the server starts returning contents again -- at which point the marker should come off. Worth noting for whoever owns the search backend: MIGRATION.md still tells users "`livecrawl` and `livecrawl_formats` still work on `POST /v1/search`... removal is targeted for 4.0.0", and we are on 3.5.0. Deprecated is not the same as broken, so either the promise or the behavior needs to move. Live suite now 45 passed / 1 xfailed instead of 1 failed.
…models (DX-835) Two review findings, both reproduced before fixing. _audit indexed spy.last[root], so a response omitting that section raised KeyError and was reported as a call failure (exit 2), aborting the remaining endpoints. Reproduced with a body containing only `metadata`. Now uses .get() and reports the absent section explicitly rather than printing a vacuous "ok" -- "nothing to walk" and "walked it, kept everything" are different claims and the output should not conflate them. _walk only considered declared model_fields, so a model configured extra="allow" would report its retained keys as dropped. Not hypothetical: Result and TaskDetailInput are both configured that way. Extras present in model_extra now count as kept, and a real drop on an ordinary model is still reported. Also widened KNOWN_WIRE_EXTRAS to accept a None label meaning "any call". `results.web[].original_thumbnail_url` is undeclared by every published spec, and keying it to one call label made the same undeclared field look unexplained as soon as another call returned it. Verified: the absent-root simulation reports instead of raising; an extra="allow" model yields 0 dropped / 2 kept; an undeclared field on WebResult is still reported. Full audit over 7 endpoints and 241 wire keys exits 0.
Contributor
|
Droid finished @tyler5673's task —— View job The PR looks solid and well-covered by tests. The main nits are making the README configuration snippets copy-paste runnable (define the API key and import |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
…_wire --help (DX-835)
Two findings from the fresh review pass.
README's five `## Configuration` snippets each constructed their own client with
`api_key_auth=key`, but no block defined `key` or imported `You`, so copy-pasting
any of them raised NameError. They read as standalone, unlike the Quick Start
continuations elsewhere in the file, which reuse an established `you` and are
obviously fragments -- that is what makes these worth fixing rather than leaving
as illustrative. Each now carries its imports and `os.getenv("YDC_API_KEY")`. One
of the five (Retries) was already touched by this branch when it gained
`timeout_ms`, so leaving it half-fixed was the worst of the available options.
scripts/audit_wire.py built its argparse description from `__doc__.splitlines()[1]`,
which raises AttributeError under `python -OO` and is brittle to any reflow of the
module docstring. Reproduced: `python -OO scripts/audit_wire.py --help` failed with
`'NoneType' object has no attribute 'splitlines'`. Replaced with a literal string,
matching how the sibling scripts/check_drift.py already does it -- that removes
both failure modes at once rather than guarding one, and keeps the two scripts
consistent.
Verified: --help works under -OO and normal mode; no `api_key_auth=key` remains in
README; all 21 README python blocks parse; the runnable/timeout_ms checker drops
from 8 self-constructed-client violations to 7, the remainder being Quick Start
continuations that never construct a client. Offline gate 450, mypy clean over 87
files, pylint 10.00/10, drift --strict clean, docs audit 0 findings, wire audit
221 keys with no unexplained drops.
Contributor
|
Droid finished @tyler5673's task —— View job Pass 2 (Validation) complete LGTM. I did not find any high-confidence, actionable issues in the changed code. |
This was referenced Sep 22, 2026
Contributor
Author
|
Superseded by #65 — the same work squashed into a single commit on a fresh branch, so the review starts from a clean history. Tree hash is identical to this PR's head (verified: |
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
1 participant
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.
Summary
Supersedes #62 and #63 — both closed only to keep the discussion readable, not
because of anything wrong with the work. Same branch, same commits, identical
diff; every review thread on both was resolved before closing.
Adds the
knowledgerequest parameter onPOST /v1/searchand the knowledgeresult models that come back with it.
Request —
knowledge="core"onsearch()/search_async()and thedeprecated
you.search.unified*shims. Normalized to lowercase like the otherenum-typed params. An invalid value raises
ValidationErrorlocally before anynetwork call, mirroring the server's
422(same fail-fast contract asextraction). Every parameter on these methods is keyword-only, so insertingknowledgemid-signature is non-breaking — hence a minor bump to 3.5.0.Response —
Results.knowledgeis an optionalList[KnowledgeResult]. Eachresult carries
type,title, andattribution, plusdescriptionand anoptional
as_ofdate fortype: answerresults.KnowledgeAttributioncarriesnameand an optionalsource_description.Two contract details drove the model design:
typeis a plainstr, not an enum. The spec says to ignore an unrecognizedvalue rather than fail on it, since a new kind may populate a different set of
fields. An enum here would turn a future server-side addition into a client
crash.
results.knowledgewhen nothing relevant is found ratherthan returning
[], so the parsed field isNone. Docs and examples iteratewith
or [].Contract grounding
Grounded only against official public surfaces:
https://you.com/docs/openapi/web-search.json— the only spec of the sevenpublished specs that mentions
knowledgeat allhttps://docs.you.com/api-reference/search/v1-search.mdhttps://docs.you.com/administration/billing.mdAlso verified against the live prod API: enum shape, the omitted-key behavior,
as_ofgenuinely optional, invalid value →422, and thatcountdoes notcap knowledge (a
count=1call still returned 2 knowledge results — knowledgehas its own documented limit of 25).
The implementation is scoped to exactly what the public spec defines: no value
beyond the one published there is accepted or modeled.
Also in this PR: drift-checker blind spot
scripts/check_drift.pycompared only top-level response properties, so anew nested field like
results.knowledgewas invisible — the checker reportedthe request-param drift but not the response drift. This is why the original
warning named only the parameter.
The response check now recurses through nested object schemas and arrays. It
falls back to the previous flat comparison when a response resolves to a
oneOfunion rather than a single object schema, so union responses are not mistaken
for empty schemas.
Recursion immediately surfaced two pre-existing gaps unrelated to Knowledge.
Both are now closed by adding the fields (
5be45e5) rather than suppressed:answerresults.webdescription,thumbnail_urlAnswerSearchResultfinance-researchoutput.sourcessnippetsFinanceResearchSourceThe answer one was live data loss. Prod returns
descriptionon 20/20 webresults and
thumbnail_urlon 16/20, and both were discarded at parse time.WebResulton the search endpoint already declared them, soAnswerSearchResultwas simply the narrower of two siblings describing the same wire shape.
The finance one was not losing data yet — two live calls at
deepandexhaustivereturned 10 sources carrying onlytitleandurl— but the specdefines
snippetsand the siblingSourcemodel on the Research API alreadydeclared it with an identical description. It was added anyway, because the
suppression holding it back can only notice the SDK catching up, never the
API starting to honour its own spec, so that gap would have stayed silent from
the API side indefinitely.
KNOWN_RESPONSE_GAPSis therefore empty, anddrift --verbosereports nodrift with an empty table — which is the real proof the gaps closed rather than
got hidden. Both suppression tables stay self-invalidating: an entry is reported
stale once the side it excuses catches up, and that path is negative-tested so it
is still live for the next gap.
Negative-tested the fix three ways: dropping
knowledgefromResultsiscaught; a stale suppression entry is reported; and a simulated spec enum gain is
caught by the enum check.
Two follow-ups hardened that recursion:
visitedwas a global cache, not a cycle breaker. It returned earlywhenever the same pydantic class reappeared, so a model used at two response
paths backed by different schemas was compared only at the first and real
drift on later branches went unreported. Reproduced before fixing, with a root
model whose two fields share a type but whose spec schemas differ by one field:
only the first path was reported.
visitedis now discarded on exit(
try/finally), so it breaks cycles without suppressing sibling branches.Checked against self-referential (
Node.child -> Node) and mutual(
A.b -> B.a -> A) schemas: both still terminate, and drift inside the cycleis still reported.
SDK declares that the spec schema at that path does not:
results.news.contentscarrieshighlightsin the SDK but not in the spec.results.web[].contentsresolves toWebContentsPost, which defineshighlights, whileresults.news[].contentsresolves to the narrowerContentsschema (html,markdown), and the SDK shares oneContentsmodelacross both. Settled against prod rather than by inspection: with
extraction_mode="highlights"news items carry nocontentsat all, and withthe deprecated
livecrawl="all"they carryhtmlonly, sohighlightsneverarrives on the news path and the spec is the accurate side. Narrowing the model
would be a breaking change for no behavioral gain, so
KNOWN_SHARED_MODEL_EXTRASrecords the gap — mirroringKNOWN_RESPONSE_GAPSand self-invalidating the same way.
Negative-tested eight ways in total:
knowledgefromResultsis caught;KNOWN_RESPONSE_GAPSentry is reported (re-checked after the tablewent empty, by injecting an entry for a field the SDK now defines);
KNOWN_SHARED_MODEL_EXTRASentry is reported (injectedhtml, whichthe spec does define at that path);
known & model_fieldsintersection got wrong;skipped;
Node.child -> Node) terminates and still reportsdrift inside the cycle;
A.b -> B.a -> A) terminates and still reports drift insidethe cycle.
scripts/check_drift.pyhad no committed test coverage at all, which is why twobugs in one function — the
visitedcache and the staleness intersection —surfaced through review rather than through a failing test, and why every
reproduction above was hand-written and then thrown away.
tests/test_check_drift.py(22 tests) now pins those rules permanently:response recursion including a sibling branch that reuses a model, cycle safety
for self-referential and mutual schemas, all three staleness cases for both
suppression tables, and the
_resolve_schema/_nested_modelhelpers. Bothregressions were confirmed to actually fail when their fix is reverted and pass
when it is restored, so they are regression tests rather than mere coverage.
Also in this PR: a live wire audit
check_drift.pycompares the published specs against the models. That is blindto a field the API returns but no spec declares — the model drops it at parse
time and every static check still agrees with itself, reporting no drift. That
is precisely how
AnswerSearchResultcame to losedescriptionandthumbnail_url: the spec does declare both, but the fact that settled it (prodactually sending them, on 20/20 and 16/20 results) only came from reading the
wire.
scripts/audit_wire.pycloses the gap. It wraps the transport, keeps the rawJSON, and walks it next to the parsed model, reporting any key no field
consumed. Across all seven endpoints that is 228 wire keys, none unexplained.
It needs
YDC_API_KEY, so it is a pre-release check rather than a CI gate;--strictexits 1 on an unexplained drop, negative-tested by emptying the knownlist and confirming it fails.
Two keys are recorded in
KNOWN_WIRE_EXTRASas observed-but-undeclared ratherthan modeled — Documented skips below explains why each is a decision rather than
an oversight.
Verification
pytest tests/(CI gate, live + perf excluded)pytest tests/test_live.py(full suite against prod)description20/20 andthumbnail_url16/20 answer web results now parse; previously all droppedcheck_drift.pywith an emptyKNOWN_RESPONSE_GAPSmypy src/youdotcom/ scripts/pylint src/youdotcom/ --disable=all --enable=E(CI gate)python scripts/check_drift.py --verbose/--strictgo build ./...clean;gofmtclean on the edited handleruv build --sdist --wheelexamples/functionTypecells vs. real annotations (typing.get_type_hints)docs/models/field table, all four AGENTS.md traps: 0 drift## Valuespages vs. code membersknowledgetests/README.mdvs.pytest --collect-onlyscripts/audit_wire.pyagainst prodresearchresponse.md) is fixedscripts/check_drift.pyregression suitedocs/,README.md,USAGE.mdvs. the copy-paste-runnable andtimeout_msrulese405ccaResponse composition is not assumed
Results.web,.newsand.knowledgeare allOptional, andresultsitselfis
Optional. Which sections a given call returns depends on the query and theparameters, and the published schema marks none of them required, so the SDK does
not assume any particular combination arrives together.
That is a deliberate design choice, and it is why nothing in this PR couples
knowledgeto the other sections:webornewsaccompanyknowledge results.
results.knowledge, never onresults.web, so they neither encode nor depend on a particular composition.if res.results:andor []—because either can legitimately be absent.
docs/models/results.mdstates the optionality explicitly and shows the guardpattern. It was previously a bare field table with no prose, unlike its sibling
searchresponse.md. The example on that page was executed verbatim against thelive API and runs clean.
Live testing during this PR saw the section mix vary across queries run with
identical parameters, including queries that returned no sections at all. That is
the behavior the optionality exists to absorb, and it is why composition is
treated as a runtime property rather than a documented guarantee: if it changes
server-side, no SDK change is required.
Two related things deliberately not done:
ordinary, correct responses, and it would assert the SDK knows the server's
intent.
sections you get when
knowledgeis set" into the docs would be wrong as soonas that changes, and nothing would catch the staleness —
check_drift.pycompares field names, enums, endpoints and servers, never descriptions.
The
Knowledgeenum docstring stays verbatim againstcomponents.schemas.Knowledge.description, per the convention that modeldocstrings mirror the published spec. The SDK-authored surfaces — the parameter
tables, both field docstrings,
_search_impl's:param knowledge:, anddocs/models/knowledge.md— describe where results land without assertinganything about the other sections.
Also worth noting: not every query yields knowledge results. The API omits
results.knowledgeentirely when nothing relevant is found, so the parsed fieldis
Nonerather than an empty list. The live tests use queries re-verifiedagainst the live API.
Surface sweep
Models, exports, request plumbing, shims, tests, docs, examples, changelog, and
version — following the AGENTS.md checklist:
Knowledge,KnowledgeResult,KnowledgeAttribution, eachwith TypedDict + Pydantic docstrings in lockstep and
optional_fieldsinserialize_model. Exported in all three places inmodels/__init__.py._build_search_request,_search_impl,search_async, andall three
SearchShimmethods.results.md,searchrequestbody.md,docs/sdks/search/README.md, anddocs/sdks/you/README.md(search section only, not the answer section).Type cells mirror the annotations including
Optional[...].[knowledge]snippet block,examples/api-example-calls.pygains a functionplus its
FUNCTIONSmenu entry.tests/test_knowledge.py, 6 live tests, 1 perfcase, and
knowledgeadded totests/test_param_normalization.pyalongsideits normalized siblings.
pyproject.toml,_version.py, anduv.lock.Documented skips
Per AGENTS.md, each skipped surface and why:
MIGRATION.mdsection — additive and non-breaking, nothing to migrate.src/youdotcom/models/searchop.py(SearchRequest) gets noknowledgefield.The published spec (
https://you.com/docs/openapi/web-search.json) defines exactlyone operation,
POST /v1/search, whose body isSearchRequestBody— there is no GEToperation and no
SearchRequestschema to sync against.SearchRequestis aquery-param model (
QueryParamMetadata(style="form", explode=True)) that the SDKnever puts on a request path (it builds
SearchRequestBody), and its fields alreadydiffer by design:
include_domainsis a comma-separatedOptional[str]here versusOptional[List[str]]on the body. Addingknowledgewould advertise a capabilitywith no public contract basis. It likewise has no
extraction, dating to 3.1.0.docs/models/searchrequest.mdis accurate against the model as it stands; its onewrong type cell (
livecrawl_formats) was fixed inbe9b065.mention knowledge, so adding a price claim would be unsupported.
WebResult.original_thumbnail_urlnot modeled — prod returns it on webresults and no published spec declares it, so
check_drift.pycannot see itfrom either side. Modeling a field observed only on the wire would make the
SDK authoritative for behavior the contract does not promise. Recorded in
KNOWN_WIRE_EXTRAS; it reads like a spec omission worth reporting upstream.FinanceResearchResponse.warningsnot modeled — prod sendswarnings: []at the top level and the sibling
research.jsondeclares it (ResearchResponsemodels it), but
finance-research.jsondeclares onlyoutput. I added thefield first;
check_drift.py --strictthen failed on exactly the standingmismatch that predicted, so it was reverted. The rule that falls out matches
snippets: add the field when that endpoint's spec declares it, record itwhen only prod does.
tests/test_research.pypins the current state so closingthe gap later is a deliberate act rather than a silent one.
One former skip was closed instead of kept: the two pre-existing nested response
gaps (
AnswerSearchResult.description/.thumbnail_urlandFinanceResearchSource.snippets) are fixed in5be45e5rather than tracked viaKNOWN_RESPONSE_GAPS, which is now empty.Two further surfaces were brought up to date in the same pass (
be9b065):tests/README.md— listed 11 of the 20 test files and carried wrong counts(Contents 12 vs 13, Answer 23 vs 25). Now lists all 21 with groups for extraction,
knowledge,
page_age, stream events and the cross-cutting suites, and counts takenfrom
pytest --collect-only. The groups sum to 450, exactly the CI-gate collection.Optionaltype-cell drift acrossdocs/models/— a mechanical scancomparing every
Typecell against its real annotation found 11 rows across 8 pageswith the same
Optional[X]↔Xmismatch this PR fixed inresults.md:contentsrequest,sourcecontrol,source,searchrequest,researchdetail,financeresearchdetail,researchrequest,researchtaskstreameventdata. Allpre-dated this branch. Each cell fit its existing padded column, so only those 11
lines changed and table alignment is preserved. The repo now audits clean.
Review notes / open questions
requestsrather than this SDK, and the search guide's optional-parametertable does not list
knowledge. Both are documentation-site changes ratherthan SDK changes, so they are handled separately from this PR.