feat(sdk): card-phase preflight planners (label hex, report filter)

[adriannoes]

During agent-seeding of the Analytics pipe (306996636), agents used execute_graphql and introspect_type / search_schema when first-class tools lacked card/phase workflows. Spec: .cursor/dev-planning/specs/archive/card-phase-ergonomics/

Stack replacing closed #277. Merge prior PRs in the stack before this one.

---

Objective

Pure SDK planners (no GraphQL, no MCP/CLI): normalize_label_color (#RGB/#RRGGBB) and validate_report_cards_filter (ReportCardsFilter shape).

Why now

Chaos-pipe hit create_label("red") and malformed report filters; validation belongs in a testable layer before tools call GraphQL.

Improvements

• Fail-fast, grep-friendly errors for agents.
• Full unit coverage without network.

[gbrlcustodio]

Review

Two pure SDK planners (normalize_label_color, validate_report_cards_filter) with full unit coverage and no network. The logic is correct and the test suite is thorough. One finding, about naming rather than behavior. The errors-as-values contract was checked against both real consumers (the CLI adapter in #282 and the MCP adapter in #284) and is the right call; only the function name is out of step with it.

Findings

1. [minor] report_filter_preflight.py:36 name reads validate_, return type is an error string

   validate_report_cards_filter(filter_obj) -> str | None returns None when valid and an error message string when invalid. The errors-as-values shape is correct: the SDK layer must stay free of surface-specific exceptions and envelopes, and both consumers adapt the string cleanly (the CLI raises typer.BadParameter, the MCP tool wraps it in build_report_error_payload). The return type is not the problem.

   The name is. This package's other validate_* functions return a normalized value (validate_page_size -> (int, err), validate_tool_id -> (id, err)) or raise (validate_positional_id). None return a bare error string, so validate_ sets the wrong expectation here. The convention that fits is already in the consuming file: report_tools.py has _blank_field_error(value, field) -> dict | None, and both new consumers named their adapters _report_filter_preflight_error and validate_report_filter_cli. A *_error noun phrase matches the return.

   Suggested rename, keeping the str | None return:

   def report_cards_filter_error(filter_obj: dict[str, Any] | None) -> str | None:
       """Return an error message if filter_obj is not a valid ReportCardsFilter, else None."""

   Call sites then read as what they do: if (err := report_cards_filter_error(f)) is not None:. The rename touches the function, its __all__, and the two consumer adapters in the stacked PRs feat(mcp,cli): phase inventory tools and CLI parity #282 and feat(mcp,cli): report filter preflight and CLI label hex #284, all unmerged, so the cost is contained to the stack.

[adriannoes]

Thanks @gbrlcustodio — agreed on the naming, and rather than renaming in place here, the rename lands in #291 together with the SDK-boundary move you proposed on #284: validate_report_cards_filter -> report_cards_filter_error, consumed by ReportService._prepared_report_cards_filter, which raises ValueError with the planner message. The errors-as-values shape stays, the name now matches the str | None return, and the two consumer adapters this rename would have touched in #282/#284 are deleted by the same move.