feat: add knowledge param and knowledge result models to search (DX-835) by tyler5673 · Pull Request #62 · youdotcom-oss/youdotcom-python-sdk

tyler5673 · 2026-09-21T19:45:13Z

Superseded by #63 and then by #64 — closed to keep the discussion readable, not because of anything wrong with the work. The branch continued unchanged; all review threads here were resolved before closing.

See #64 for the current description, verification, and review record.

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.

factory-droid · 2026-09-21T19:45:27Z

Droid finished @tyler5673's task —— View job

Validated 3 review comments for PR #62. The only requested changes are tightening drift-checker staleness detection, removing a misleading “alongside web/news” doc claim, and making the knowledge live test forward-compatible with future result types.

… (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.

factory-droid · 2026-09-21T20:09:00Z

Droid finished @tyler5673's task —— View job

The Knowledge request/response surface looks well-integrated and well-tested. The main issue is the new README snippet, which is not copy-paste runnable and should follow the repo’s documented example conventions.

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.

factory-droid · 2026-09-21T20:23:48Z

Droid finished @tyler5673's task —— View job

The PR looks well-scoped and well-tested for adding the knowledge request/response surface. The main remaining issue I see is a docs type-cell mismatch in Results that should be corrected to match the actual Optional[List] annotations.

tyler5673 · 2026-09-21T20:24:11Z

Review response

Eight droid-review findings across five review rounds were evaluated: 7 fixed, 1 rejected, each with a reply in its thread.

Round 1 — fixed in e64094a:

scripts/check_drift.py — _load_spec() now raises DriftError on a stale cached copy with a rm hint instead of silently trusting it. Pre-existing on main, surfaced by this PR's drift run.
tests/test_live.py — replaced the hard kr.type == "answer" pin with the conditional the spec states (type is required on every result; description is required only on type: answer), so the test no longer breaks when the server adds a kind.

Round 1 — rejected:
3. src/youdotcom/models/knowledge.py:8 — left as-is. AGENTS.md pins this exact docstring to the spec's own Knowledge.description, which is verbatim Requests knowledge results alongside web and news search.; SearchRequestBody.properties.knowledge carries no description of its own (it $refs the schema). Re-confirmed against the live spec. The SDK-layer guidance the finding asked for does exist — it lives on the parameter (all three docs tables and, since e8fa8d6, both SearchRequestBody field docstrings), which is where the docs convention puts it.

Round 2 — fixed in 01986d1:
4. README.md — the knowledge snippet used client = You(...), which the surrounding examples never define; now from youdotcom import You + client = You(api_key_auth="YOUR_API_KEY"), matching the package docs. Verified every code example in the README is syntactically parseable and that no undefined-name references remain.

Round 3 — fixed in 98c1fc4:
5. docs/models/results.md — the web and news type cells said List[...] while the annotations are Optional[List[...]]. Pre-existing on main, but adding the knowledge row re-emitted both lines, and AGENTS.md requires every row of a touched model page to match its annotation. Verified mechanically rather than by eye: all 25 Type cells across the four model pages this PR touches now match their real annotations via typing.get_type_hints.

Round 4 — fixed in e8fa8d6:
6. docs/models/searchrequestbody.md — cross-table identity: the three docs tables carried the expanded knowledge description while both field docstrings stopped after the first sentence. Expanded the docstrings rather than simplifying the tables, since the tables are the established richer surface (cf. include_domains, extraction); all five surfaces are now byte-identical.
7. tests/test_live.py — assert kr.description → assert kr.description is not None, because description is Optional[str] and truthiness would additionally enforce non-empty. Left truthiness on type/title/attribution[].name deliberately: those are required str fields where is not None is vacuous and non-empty is the meaningful claim.

Round 5 — fixed in e405cca:
8. docs/models/knowledgeresult.md — the as_of note told readers to call datetime.strptime(...) without showing an import, so following it raised NameError. Replaced with a fenced block carrying from datetime import datetime, matching the pattern docs/models/webresult.md already uses for its page_age datetime note. The block also guards the optional section (res.results.knowledge or []) and uses is not None rather than truthiness. The suggested one-line inline form was not used, to keep code as code and the two sibling notes consistent.

Pre-existing drift found while auditing — now fixed in be9b065. Widening the type-cell scan to every field table in docs/models/ found 11 more rows across 8 pages with the same Optional omission (contentsrequest, sourcecontrol, source, searchrequest, researchdetail, financeresearchdetail, researchrequest, researchtaskstreameventdata). All pre-dated this branch. Fixed rather than deferred: every cell fit its existing padded column, so only those 11 lines changed and no table alignment moved. The same commit brings tests/README.md up to date — it listed 11 of the 20 test files and carried wrong counts for Contents (12 vs 13) and Answer (23 vs 25); its group counts now sum to 425, exactly the CI-gate collection.

The audit is heading-aware, so it also covers multi-model pages such as answerresponse.md whose tables sit under a class-name heading rather than a single fields heading. It checks all four traps AGENTS.md names — Optional↔X, datetime↔date, bool↔int, str↔int — across 151 field-table rows: 0 findings remaining. The 15 enum value pages were also diffed against their code members and match, the one apparent hit being contentsformats' intentional metadata _(deprecated)_ annotation. Every Python block in docs/, README.md and USAGE.md was additionally checked against the copy-paste-runnable and timeout_ms rules; that sweep is what surfaced the Retries example fixed in e405cca.

The one surface deliberately left without knowledge is SearchRequest (src/youdotcom/models/searchop.py). The published spec defines a single operation, POST /v1/search, whose body is SearchRequestBody — there is no GET operation and no SearchRequest schema to sync against, and the SDK never puts that model on a request path. Its fields also differ by design (include_domains is a comma-separated Optional[str] there versus Optional[List[str]] on the body). Adding the field would advertise a capability with no public contract basis. docs/models/searchrequest.md is accurate against the model as it stands.

Response composition is not assumed. Results.web, .news and .knowledge are all Optional, and the published schema marks none of them required, so the SDK does not assume any particular combination arrives together. No docstring, doc page, example or live test asserts that web or news accompany knowledge results. Live testing during this PR saw the section mix vary across queries run with identical parameters, which is the behavior that optionality exists to absorb — composition can change server-side without an SDK change.

CI on e405cca: 7/7 green (4 Python versions, build-check, drift-check, droid-review). Full gate locally: 425 offline tests, 6 live knowledge tests against prod, mypy clean, pylint 10.00/10 on the CI errors-only gate, check_drift.py --strict no_drift, build OK, and the docs audit above at 0 findings.

`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.

factory-droid · 2026-09-21T21:28:37Z

Droid finished @tyler5673's task —— View job

The PR looks solid and well-tested for adding the knowledge request/response surface. I only found one docs-surface mismatch around knowledge descriptions and one live-test assertion that may incorrectly enforce non-empty strings; no security issues stood out in this diff.

…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.

factory-droid · 2026-09-21T21:46:52Z

Droid finished @tyler5673's task —— View job

No high-confidence, actionable issues found in this diff; the knowledge request/response surface and drift-checker changes look consistent and well-covered by tests/docs.

…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.

factory-droid · 2026-09-21T22:33:14Z

Droid finished @tyler5673's task —— View job

The knowledge request/response surface looks solid and well-tested. I only found one small docs footgun where the as_of parsing note references datetime without showing an import.

…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.

factory-droid · 2026-09-21T22:46:04Z

Droid finished @tyler5673's task —— View job

Phase 2 (validator): approved

No high-confidence, actionable issues found in this diff. The knowledge request/response surface and drift-checker recursion changes look consistent and well-covered by tests and docs.

tyler5673 · 2026-09-22T01:14:37Z

Superseded by #63 — same branch, same 7 commits, identical diff. Closing this one only because the review discussion here grew hard to follow; #63 starts with a clean thread. All 8 review threads on this PR were resolved, and every finding is addressed in the commits that #63 carries.