Skip to content

fix(datasource-zendesk): coerce id filter values to integers for PK lookups#331

Open
christophebrun-forest wants to merge 3 commits into
mainfrom
fix/zendesk-id-filter-coercion
Open

fix(datasource-zendesk): coerce id filter values to integers for PK lookups#331
christophebrun-forest wants to merge 3 commits into
mainfrom
fix/zendesk-id-filter-coercion

Conversation

@christophebrun-forest

@christophebrun-forest christophebrun-forest commented Jul 13, 2026

Copy link
Copy Markdown
Member

Problem

The agent parses Number values through to_f, so id = N / id IN [...] filters reach the Zendesk datasource as floats (e.g. 123.0). The PK-lookup short-circuit (extract_id_lookup) forwarded those straight to Zendesk's REST endpoints:

  • GET /tickets/show_many?ids=123.0 is rejected by Zendesk. Its error body embeds a validation regex whose backslashes are invalid JSON, so parsing the response also throws — surfacing as:
    APIError: Zendesk API call failed: fetch_tickets_by_ids:
    JSON::ParserError: invalid escape character in string:
    '\A\s*(?:(?:\d+|suspended|deleted' ...
    
  • Even when a call succeeds, the id-keyed result map ({ 123 => ticket }) is looked up with a float key (123.0) → miss → records silently dropped.

This was hit in real conditions by the workflow fallback-inbox flow, which lists client records via id IN [recordId].

Fix

Normalise id values to Integer in extract_id_lookup — the single choke point shared by Ticket (fetch_tickets_by_ids) and User/Organization (find_one) — before any REST call. Handles float and numeric-string inputs, leaves non-coercible values untouched.

Why existing tests missed it

The specs built condition trees with raw integers (Leaf.new('id', 'in', [1, 2])), bypassing the agent parser and its to_f casting. Added regression specs that pass float ids through the datasource for both the Ticket IN/EQUAL paths and the User find_one path.

Validation

  • 245 examples, 0 failures (full Zendesk datasource suite), coverage 99.33%
  • RuboCop clean
  • Confirmed against the real production stack trace

Scope note

The underlying to_f cast for Number columns lives in the shared agent parser (condition_tree_parser.rb) and affects all datasources; this PR fixes it locally where it breaks Zendesk. A broader look at the parser's numeric coercion for primary keys is worth a separate ticket.

🤖 Generated with Claude Code

Note

Coerce id filter values to integers in Zendesk datasource PK lookups

Adds a normalize_id helper to BaseCollection that converts values like 1.0 or "1" to integers using Integer(value, exception: false), falling back to the original value on failure. The extract_id_lookup method now applies this normalization for both EQUAL and IN id filters before passing ids to Zendesk API calls.

Macroscope summarized a2eff83.

The agent parses Number values through `to_f`, so `id = N` / `id IN
[...]` filters reach the datasource as floats (e.g. `123.0`). The
PK-lookup short-circuit forwarded those to Zendesk's REST endpoints, so
`/tickets/show_many?ids=123.0` was rejected -- the error body embeds a
validation regex that then fails JSON parsing, surfacing as APIError/500
-- and the id-keyed result map missed even when the call succeeded.

Normalise id values to Integer in `extract_id_lookup`, the single choke
point shared by Ticket (fetch_tickets_by_ids) and User/Organization
(find_one).

Surfaced by the workflow fallback-inbox flow, which lists client records
via `id IN [...]`; existing specs built condition trees with raw
integers and never exercised the parser's float casting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
christophebrun-forest and others added 2 commits July 13, 2026 15:16
Integer(123.9) truncates to 123, so a fractional id filter would match
an unrelated record instead of nothing. Only normalise values whose
integer conversion equals the original; leave fractional floats as-is.

Addresses PR review feedback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Revert normalize_id to the minimal integral coercion. The fractional
branch guarded an input the agent parser never produces (integer PKs
yield integral floats via to_f), and its spec asserted an empty result
that the real client would not return -- a fractional id still hits
show_many and raises the same APIError this fix addresses. Keep only
what is strictly necessary.

Co-Authored-By: Claude Opus 4.8 <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.

2 participants