Skip to content

docs(v4): document GetKeywordsSuggestion contract from official docs (closes #435) - #444

Merged
axisrow merged 4 commits into
mainfrom
feat/milestone-22-investigate-435-keywords-suggestion
May 28, 2026
Merged

docs(v4): document GetKeywordsSuggestion contract from official docs (closes #435)#444
axisrow merged 4 commits into
mainfrom
feat/milestone-22-investigate-435-keywords-suggestion

Conversation

@axisrow

@axisrow axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/GetKeywordsSuggestion and recorded the documented request shape in direct_cli/v4_contracts.py:GetKeywordsSuggestion.

Findings

  • Request body: { method, param: { Keywords: [<utf-8 strings>] } }.
  • Keywords is the only documented param field — no GeoID / MinusKeywords.
  • Response: array of up to 20 suggestion strings.
  • Consumes API points; error_code=152 if insufficient.

Changes

Field Before After
param_shape PARAM_UNDOCUMENTED PARAM_OBJECT
source_status SOURCE_UNDOCUMENTED SOURCE_DOCS
live_probe_allowed False True
example_param absent {"Keywords": ["холодильник", "камера"]}
notes absent Docs-verified 2026-05-28 ...

No CLI command is added in this PR — implementation is gated by a future follow-up issue.

Test plan

  • pytest tests/test_v4_live_contracts.py tests/test_comprehensive.py — 21 passing, 11 skipped (live tests).

Closes #435. Part of milestone 22.

🤖 Generated with Claude Code

Closes #435.

Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/GetKeywordsSuggestion
and recorded the documented request shape in v4_contracts.py.

- param shape: PARAM_UNDOCUMENTED -> PARAM_OBJECT
- source_status: SOURCE_UNDOCUMENTED -> SOURCE_DOCS
- live_probe_allowed: False -> True (READ-safe, requires API points)
- example_param: {"Keywords": ["холодильник", "камера"]}
- notes: explicit Docs-verified 2026-05-28 stamp; mentions API points
  cost and error_code=152 on insufficient quota.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 28, 2026 16:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@axisrow

axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown

Claude finished @axisrow's task in 3m 25s —— View job


Review — focused on critical issues

  • Read trigger comment
  • Inspect the diff (git show HEAD -- direct_cli/v4_contracts.py)
  • Read direct_cli/v4_contracts.py + direct_cli/v4/__init__.py + tests/test_v4_contracts.py
  • Trace consumers of live_probe_allowed, param_shape, and example_param
  • Cross-check that no current CLI command would be broken by the contract flip

Verdict

No critical issues found. This is a metadata-only update to the v4 contract registry. There is no CLI command bound to GetKeywordsSuggestion yet (grep confirms only the registry/notes mention it), so the change is purely descriptive — no new network calls, no data-loss surface, no regression risk in production.

Things I checked and ruled out

  • live_probe_allowed: False → True is not a live-traffic change. The flag is only consumed by tests/test_v4_contracts.py (the SOURCE_CONFIRMED_LIVE assertion set at L61–70, which does not include SOURCE_DOCS entries like this one) and by the registry validator at direct_cli/v4_contracts.py:639 (which requires example_param to be set when live_probe_allowed=True — satisfied). No automated probe runner reads this field today, so the flip cannot silently start spending API points.
  • PARAM_UNDOCUMENTED → PARAM_OBJECT hardens validation, but safely. Previously call_v4 (direct_cli/v4/__init__.py:36–54) would emit a soft warning for this method's param shape; after this PR a non-dict param will produce a hard click.UsageError. There are no current callers, so nobody breaks — but the follow-up CLI command must build {"Keywords": [...]} exactly.
  • Registry invariants pass by inspection. validate_v4_contract_registry requires example_param for live_probe_allowed=True (set), safety in {READ, MIXED} (READ ✓), and a non-empty login_placement (✓).
  • No SOURCE_CONFIRMED_LIVE set drift — the PR uses SOURCE_DOCS, which is correct since no live probe has been recorded yet, and it does not touch the hard-coded confirmed-live set in tests/test_v4_contracts.py:44–52.
  • File encoding is UTF-8 (file confirms) — Cyrillic example tokens are safe.

One soft observation (not a blocker, not a nitpick — a now-incorrect factual claim in code)

direct_cli/v4/__init__.py:41 still cites GetKeywordsSuggestion as the canonical undocumented read-class example:

# For read-class undocumented methods (e.g.
# GetKeywordsSuggestion) a soft warning is acceptable.

After this PR that's no longer true — GetKeywordsSuggestion is now PARAM_OBJECT / SOURCE_DOCS, and any other still-undocumented read method should take its place in the comment (or the example can simply be dropped). Not a functional bug, but mentioning because future readers will be misled by a stale reference to this exact method.

Fix this →

Things I did not (and could not) verify

  • I did not fetch yandex.ru/dev/direct/doc/dg-v4/reference/GetKeywordsSuggestion from this environment, so the accuracy of the documented shape (Keywords-only param, ≤20 suggestions, error_code=152) is trusted from the PR description. The shape matches the historical v4 contract shape pattern used elsewhere in this registry, so nothing reads as suspicious.

• Branch: feat/milestone-22-investigate-435-keywords-suggestion

axisrow added 2 commits May 29, 2026 01:30
…ment

After this PR moves GetKeywordsSuggestion from PARAM_UNDOCUMENTED/SOURCE_DOCS,
the v4/__init__.py:41 comment citing it as the canonical "read-class
undocumented" example is no longer accurate. Drop the specific example and
add a defensive helper in test_v4_runtime_shape.py that skips two
(PARAM_UNDOCUMENTED + SAFETY_READ) behaviour tests when no method matches
in the registry — both currently skip but will re-engage automatically when
a new undocumented-read entry is added.
@axisrow

axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown

Claude finished @axisrow's task in 2m 4s —— View job


Re-review — focused on critical issues

  • Inspect current diff vs origin/main
  • Re-check direct_cli/v4_contracts.py change (GetKeywordsSuggestionPARAM_OBJECT / SOURCE_DOCS / probe-allowed)
  • Re-check direct_cli/v4/__init__.py (stale GetKeywordsSuggestion reference dropped by 8c924ae — matches my earlier soft observation)
  • Re-check tests/test_v4_runtime_shape.py (_first_method_with_shape_and_safety_or_skip helper)
  • Verify registry invariants still hold
  • Inventory remaining PARAM_UNDOCUMENTED entries

Verdict

No critical issues — safe to merge.

This is still a metadata-only contract update plus a defensive refactor of two registry-driven tests. No new network calls, no CLI binding, no production code path changes shape.

What I verified

  1. Registry invariants pass. validate_v4_contract_registry requires example_param when live_probe_allowed=True (✓ {"Keywords": ["холодильник", "камера"]}), safety in {READ, MIXED} (✓ READ), login_placement non-empty (✓), param_shape and source_status in the valid sets (✓).
  2. Stale comment cited in my previous review is fixed (direct_cli/v4/__init__.py:40-41). Generic wording now — no specific example.
  3. Cyrillic example is safe. Source file is UTF-8; values pass through tapi_yandex_direct JSON serialization the same way as the rest of the v4 registry.
  4. No production CLI command is bound yet — the param_shape flip (which would turn a soft warning into a hard UsageError for malformed param on this method) has no callers to break.

One non-blocking observation (test-coverage gap, not a bug)

After this PR, the v4 registry has zero PARAM_UNDOCUMENTED + SAFETY_READ methods. The remaining undocumented entries are all WRITE/DANGEROUS:

  • PayCampaignsByCard (DANGEROUS)
  • DeleteOfflineReport, DeleteReport, AdImageAssociation (WRITE)

That means test_call_v4_warns_on_undocumented_read_method_but_proceeds and test_call_v4_undocumented_still_rejects_hard_errors will both skip at runtime via the new _first_method_with_shape_and_safety_or_skip helper. The skip is intentional and self-documented ("behaviour path is currently unreachable") — but it does leave the soft-warn branch in direct_cli/v4/__init__.py:36-54 (warn-and-proceed for undocumented-shape READ methods) without any test coverage until a future method re-populates that cell.

Two reasonable follow-ups (either, neither, both — author's call):

  • Replace those two skipped tests with a fixture/synthetic contract injected into V4_METHOD_CONTRACTS so the code path stays under test regardless of registry contents, or
  • For test_call_v4_undocumented_still_rejects_hard_errors specifically: that test actually exercises the hard_errors filter at direct_cli/v4/__init__.py:32-34, which runs before the safety branch — so any registered method would do. The _or_skip guard is overly cautious there.

Neither is a release blocker. The skips are honest about what they're doing, and if/when a new undocumented READ method lands, both tests come back online automatically.

Fix this →

What I did not verify

I did not fetch the upstream Yandex docs page from this environment, so the documented shape (Keywords-only param, ≤20 suggestions, error_code=152) is trusted from the PR description and the notes field. The shape is consistent with the rest of the v4 contract registry.

--- • Branch: feat/milestone-22-investigate-435-keywords-suggestion

The hard-error filter in call_v4 runs before the safety branch, so the
PARAM_UNDOCUMENTED + SAFETY_READ pairing is not needed for this test —
any registered READ method exercises the path. Drops the unnecessary
skip introduced by 8c924ae for this one test.
@axisrow

axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@claude review. Focus on critical issues only.

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown

Claude finished @axisrow's task in 1m 27s —— View job


Re-review — focused on critical issues

  • Diff PR vs origin/main
  • Re-verify direct_cli/v4_contracts.py change
  • Re-verify direct_cli/v4/__init__.py stale-comment fix
  • Re-verify tests/test_v4_runtime_shape.py helper
  • Confirm registry invariants
  • Post verdict

Verdict

No critical issues — safe to merge.

Still metadata-only at the production-code level. The two follow-up commits since my last pass (8c924ae, 77ad96e) cleanly addressed both soft observations from previous rounds; nothing new introduces a regression risk.

What I verified on this pass

  1. Diff scope is unchanged: three files, +43/-8, all in the v4 contract registry / its tests / one stale comment. No CLI command is added or bound to GetKeywordsSuggestion yet, so the PARAM_UNDOCUMENTED → PARAM_OBJECT flip (which converts the soft warn into a hard UsageError for non-dict param) has zero current callers to break.
  2. Registry invariants still pass by inspection (direct_cli/v4_contracts.py:626-650):
    • param_shape ∈ valid set ✓ (PARAM_OBJECT)
    • safety ∈ valid set ✓ (SAFETY_READ)
    • source_status ∈ valid set ✓ (SOURCE_DOCS)
    • login_placement non-empty ✓
    • live_probe_allowed=True ⇒ safety ∈ {READ, MIXED} ✓ (READ)
    • live_probe_allowed=Trueexample_param is not None ✓ ({"Keywords": [...]} set)
  3. Soft-warn comment in direct_cli/v4/__init__.py:36-41 no longer cites GetKeywordsSuggestion — generic wording, won't go stale again.
  4. Test coverage gap from my prior review is partially closed. test_call_v4_undocumented_still_rejects_hard_errors now uses _first_method_with_shape_and_safety(PARAM_ARRAY, SAFETY_READ) and patches validate_v4_body_shape to inject mixed errors (tests/test_v4_runtime_shape.py:154-163). Because the validator is mocked, the hard-error gate at direct_cli/v4/__init__.py:32-34 is exercised on every run regardless of registry contents. ✓
  5. test_call_v4_warns_on_undocumented_read_method_but_proceeds will skip at runtime because there's no PARAM_UNDOCUMENTED + SAFETY_READ entry left (remaining undocumented entries are PayCampaignsByCard DANGEROUS and three WRITE-class entries). The skip is intentional and self-documented in the helper docstring (tests/test_v4_runtime_shape.py:41-55) — the soft-warn code path is unreachable from real callers until a new method re-populates that cell, at which point the test re-arms automatically. Honest skip, not silent dead code.
  6. Cyrillic example tokens are safe — file is UTF-8 and these strings only travel through tapi_yandex_direct's standard JSON serialization, identical to every other example_param in the registry.

What I did not verify

I did not fetch yandex.ru/dev/direct/doc/dg-v4/reference/GetKeywordsSuggestion from this environment, so the documented contract details (Keywords-only param, ≤20 suggestions, error_code=152 on points exhaustion) are trusted from the PR description and notes field. The shape is structurally consistent with the rest of the v4 registry and the previous review's verification of the same field, so nothing reads as suspicious.


• Branch: feat/milestone-22-investigate-435-keywords-suggestion

@axisrow
axisrow merged commit 980be48 into main May 28, 2026
6 checks passed
@axisrow
axisrow deleted the feat/milestone-22-investigate-435-keywords-suggestion branch May 28, 2026 18:53
axisrow added a commit that referenced this pull request May 28, 2026
…449)

* test(v4): skip undocumented-write shape test when registry has no such method

After #444-447 documented AdImageAssociation/DeleteReport/DeleteOfflineReport/
GetKeywordsSuggestion, no contract remains with param_shape=undocumented and
safety=write, so the hard-fail lookup raised AssertionError. Mirror the existing
undocumented-read test and use the _or_skip variant: the runtime guard still
exists, the path is just unreachable from real callers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(audits): add dated API coverage snapshot (closes #428)

Records main @ e9c7ea5 parity as a reference point: v5 WSDL 29 services/112 ops
(strict parity), reports 8 types/84 fields, optional-field audit 3215 supported/
26 not_applicable, 23/32 v4 contracts CLI-wired. Documents that the four
formerly-undocumented v4 methods are now docs-verified and that GetCreditLimits/
CheckPayment have CLI commands.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: axisrow <axisrow@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

v4 docs-drift: investigate GetKeywordsSuggestion contract

3 participants