@codex @claude review

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

Claude Code Review

• Read PR diff
• Read changed files (audiencetargets.py, test_dry_run.py, test_wsdl_parity_gate.py, audit)
• Cross-check WSDL/audit invariants (PriorityEnum = LOW/NORMAL/HIGH per tests/wsdl_cache/imports/general.xsd:143-148, audit math −2/+2 checks out)
• Post review feedback

Overall: small, focused, on-spec — classifies the two optional WSDL rows correctly, enum matches the schema, audit + parity-gate map both updated, dry-run test covers happy path and rejection. Two follow-ups worth raising; nothing blocking.

What's right

• click.Choice(["LOW","NORMAL","HIGH"], case_sensitive=False) matches PriorityEnum exactly (tests/wsdl_cache/imports/general.xsd:143-148). ✓
• priority.upper() is necessary, not redundant — Click's case_sensitive=False controls matching but returns the user's original casing, so the explicit normalization is what guarantees the API receives the canonical HIGH even when the user typed high. The happy-path test was cleverly updated to pass "high" (lowercase) and assert "HIGH" in the payload, exercising both the choice match and the normalization in one shot (tests/test_dry_run.py:3739,3747).
• Audit math: missing_followup −2, supported +2 — matches the two rows reclassified. Parity-gate OPTIONAL_FIELD_CLI_OPTIONS entries added for both fields (tests/test_wsdl_parity_gate.py:824-825). ✓
• New rejection test (tests/test_dry_run.py:3751-3763) confirms Click-level rejection of --priority MAX — silent-data-loss invariant honored.

Issues / nits

1. (Follow-up) Same --priority on audiencetargets set-bids is still untyped — silent garbage path remains
direct_cli/commands/audiencetargets.py:167:

@click.option("--priority", help="Strategy priority")

The setBids request uses the same PriorityEnum (cached WSDL tests/wsdl_cache/audiencetargets.xml:43), so audiencetargets set-bids --priority foo still forwards "foo" verbatim and there's no .upper() either — --priority high would also reach the API as "high". The PR title scopes this to add, so leaving setBids alone is defensible, but it's the exact class of bug invariant #2 (Silent data loss) in CLAUDE.md is meant to catch. Worth a separate issue or a one-line extension to this PR. Fix this →

2. (Nit) --bid help text now drifts from sibling commands
direct_cli/commands/audiencetargets.py:104 changed "Context bid in micro-rubles" → "ContextBid value in micro-rubles". The sibling set-bids --context-bid on line 166 still reads "Context bid in micro-rubles", as do most other commands. Surfacing the raw WSDL field name in --help text is more technical but less user-friendly than the prior wording, and the inconsistency between add and set-bids is a small papercut for anyone scanning --help. Consider reverting the wording change or applying it everywhere for consistency.

3. (Nit) if priority: vs. if priority is not None:
direct_cli/commands/audiencetargets.py:138 uses truthiness. Click's Choice never returns an empty string for an opted-out value (it returns None), so functionally equivalent today, but is not None is the convention used immediately above for bid (line 136) and reads more clearly. Not a behavior change — purely style.

· branch codex/issue-302-audiencetargets-add-bids

Codex Review: Didn't find any major issues. Keep it up!

Feedback classification after review-cycle:

• audiencetargets set-bids priority typing: follow-up, not blocking this PR. Issue audiencetargets add: ContextBid and StrategyPriority typed flags #302 is scoped to audiencetargets.add WSDL rows ContextBid and StrategyPriority, and this PR keeps the accepted add-only scope focused.
• bid help wording: no-op for this PR. It names the exact WSDL field covered by the issue and does not affect CLI contract or payload behavior.
• if priority vs is not None: no-op style nit; Click Choice cannot produce an empty opted-in value here.

No blocking/actionable feedback remains for #302.