Skip to content

feat(datasource intercom): search filters and per-endpoint operator tables (lot 2) - #384

Merged
christophebrun-forest merged 10 commits into
feat/datasource-intercomfrom
feat/prd-1118-search-filters
Sep 3, 2026
Merged

feat(datasource intercom): search filters and per-endpoint operator tables (lot 2)#384
christophebrun-forest merged 10 commits into
feat/datasource-intercomfrom
feat/prd-1118-search-filters

Conversation

@christophebrun-forest

@christophebrun-forest christophebrun-forest commented Sep 2, 2026

Copy link
Copy Markdown
Member

Lot 2 of the Intercom datasource (PRD-1118), on top of lot 1 (#383). Base is the integration branch feat/datasource-intercom, not main.

Turns a list you can only browse into a list you can query — and keeps the rule lot 1 was built on: a page that looks filtered without being filtered is the one answer this datasource must not give. Every condition it cannot express exactly is refused by name, before any request leaves the process.

The three findings worth reading the diff for

1. today returned nothing, and would have shipped that way. Intercom truncates a date search to the day, at the UTC boundary — measured, against its own documentation, which promises the workspace's timezone. > V answers from the start of the day after V; < V answers before the start of V's own day. So the pair of bounds the toolkit rewrites today into reads as "from tomorrow" and "before today": no rows, to the most ordinary filter there is, with nothing in the answer saying why. DayBounds moves each bound to the boundary that makes Intercom answer the day the filter named. A caller in UTC gets exactly that day; a caller in another timezone gets the UTC days their window overlaps — up to a day wider at each end — and is told once per filter.

2. PRD-989 is not blocking this lot, measured. On a Date column declaring greater_than + less_than and nothing else, 20 operators are published (before, after, today, yesterday, past, future, the whole previous_* family) and none is refused by Rules. Adding equal republishes in, which the validator then refuses. So a date column here carries the two bounds alone, even where the endpoint accepts six — what stays out is an equality on an instant, which a day-granular filter could not have honoured anyway. A spec asserts the invariant PRD-989 says nothing checks: for every column of every endpoint, everything the agent publishes from what the column declares is allowed by its own validator.

3. The field list is measured, not documented. /tickets/search refuses company_id with invalid_field although every ticket carries one. The source of truth is a committed table, one row per column, each carrying its provenance (measured / spec); bin/probe_search_fields measures it against a real workspace, one search per (field, operator) cell, and prints what the table promises that Intercom refuses. Every filter_operators is derived from that table — no collection writes one by hand, so a column cannot advertise a filter the translator would refuse.

What is in it

  • query/search_fields.yml + Query::SearchFields — the table and its loader, validated at boot: an unknown operator, a type nothing can send, a column filed as both filterable and refused all fail loudly.
  • bin/probe_search_fields — the measurement, excluded from the gem.
  • Query::OperatorTable, Query::FilterValue, Query::DayBounds, Query::ConditionTreeTranslator — the spelling, the wire format, the day rule, and the walk.
  • Nesting depth (2) and group size (15) refused before the request leaves, with messages naming what to simplify. Past either, Intercom answers a 400 naming neither the limit nor the part of the filter that reached it.
  • CursorCollection gained a fourth route: no condition walks the listing, id equals X reads the record endpoint, anything else is translated and walked through the search. Counting follows the same path — total_count is exact on a search too, so a filtered count stays one request over the whole filtered set.
  • Free-text search on IntercomConversation through ~ on source.body, folded into the condition tree rather than added to the query, so the nesting limits are checked over the whole of it. Whole words, not substrings — the README says so.
  • The two findings from lot 1's review: the id in [...] page window (page 2 rendered page 1's rows) and default_pk_sort? reading a symbol-keyed false as absent (an explicit ?sort=-id was taken for the agent's default, so the "Intercom ignores a sort" warning never fired).
  • README section: what is filterable, what is not and why, one reason per column — with a spec checking it against the table rather than trusting it to stay true.

What stays refused, and not temporarily

The columns a ticket derives from its parts (closed_at, closed_by_name, last_reply_at, last_responder_name, last_responder_type), the account of a ticket, the tag names and contact identity of a conversation, absence (present / blank / missing, which Intercom's search has no operator for), group-by on either cursor collection, and every ticket attribute — R7, recorded in the table: an attribute is filtered through an id that differs per ticket type, so a union column has no single id to translate to.

Sort: no column of either collection becomes sortable, and that is the correct outcome. Neither search endpoint takes a sort, and one sent to /conversations/search is silently ignored.

Fixed in review

Three commits on top of the lot, each a defect the review turned up and none of them cosmetic.

id filtered next to anything else was refused, and that broke the record detail. The key is the one column whose Forest operators are not derived from the measured table: add_column writes equal and in on it by hand, the toolkit refusing a collection whose key carries neither. No row declared it, so the schema advertised a filter the translator had nothing to write. A bare id equals X never noticed, reading the record endpoint before the translator. Nested in an and it did — and a permission scope turns every record detail into id equals X and <the scope>, which the record endpoint cannot answer since the ids name a wider set than the scope does. A scope on either cursor collection therefore broke the record detail, the count and the CSV export. id is now a row of the table on both endpoints, as spec, and out of the candidates.

The invariant spec could not have caught it: it walks the table, and the gap was the one column no row of the table is derived from. It now walks the schema instead, and names the offending column and the endpoint that cannot translate it. Verified failing without the row.

A date filter was read in the timezone of the server, not of the caller. FilterFactory.get_previous_condition_tree writes the bounds of a previous period with strftime('%Y-%m-%d %H:%M:%S'), carrying no offset at all, and Time.parse read them wherever the process happened to run. Because those bounds sit on a midnight, any offset moves them onto another UTC day once DayBounds truncates. Measured for a Europe/Paris caller asking for the previous month: the filter names 31 July, Intercom is asked from 1 August — a whole day of rows beside the ones named, on a UTC server and a New York one alike. Reachable through the chart route, which builds the previous-period filter and counts through aggregate, a bare Count being exactly what these collections answer.

Time.parse stays, and only for what it refuses: it raises on a string naming no date, where Time.zone.parse answers today. A filter on last tuesday coming back as a filter on today is the one answer this datasource must not give. A value carrying an offset is untouched — every operator the toolkit rewrites into bounds arrives as UTC ISO8601 and reads the same in any zone. The timezone moved out to CallerZone, for the reason DayBounds was split off already.

An overflowed cast raised instead of being refused. Infinity or NaN on a date column made to_i raise a FloatDomainError naming a float where the operator asked for a date. The number branch already refused both, and said why; only the date guard was missing.

Not taken, and deliberately: an empty Or branch (match_none) is still refused rather than answered as zero rows — the toolkit neutralises an empty And in intersect and has no equivalent for Or, so this is a decision to record rather than a fix to slip in. And bin/probe_search_fields still records a 429 or a 401 as a refused operator; --out keeps the code, print_diff does not show it. One reported High was a false positive: Date.parse raises Date::Error, which is an ArgumentError, so an impossible calendar date was already refused by the existing rescue.

Before merging

  • Run bin/probe_search_fields against the customer's workspace and commit what it settles: every row still marked spec is a candidate, and so is whether POST /conversations/search honours display_as=plaintext — if it does not, a filtered read comes back as HTML where an unfiltered one comes back as text (R10).
  • R7: coded as option 1 (attributes display-only). One collection per ticket type if the customer says the trade is worth it — the ids are already kept per type.

Checks

372 examples, 0 failures, 100% line coverage on the package. bundle exec rubocop clean at the repository root, 896 files. The suite passes identically under UTC, Europe/Paris, America/New_York, Asia/Tokyo and Pacific/Auckland — the date specs no longer depend on the timezone the process runs in.

🤖 Generated with Claude Code

Note

Add search filters and per-endpoint operator tables to Intercom datasource

  • CursorCollection translates Forest condition trees and free-text searches into Intercom's search DSL instead of refusing all non-id filters.
  • Collection schemas advertise supported filter operators for each field using per-endpoint operator tables.
  • Added bin/probe_search_fields CLI to probe Intercom endpoints, measure supported operators, and output YAML evidence for the search field registry.
  • Date and timestamp filter values are converted using the caller-supplied timezone via CallerZone, with UTC fallback.
  • Risk: CursorCollection previously refused all non-id filters with a listing-endpoint error; it now attempts to translate them through the search endpoint.

Macroscope summarized af19c4e.

Two findings from the lot 1 review, both places where a cursor
collection answers something other than what it was asked for.

An `id in [...]` read returned every record it fetched whatever
page was asked for, so page 1 and page 2 of a related-record list
rendered the same rows. The window is now cut out of the ids
before they are read, which also spares the requests the
discarded records cost: Intercom reads them one request each.

`default_pk_sort?` read a symbol-keyed `false` as an absent key,
so an explicit `?sort=-id` was taken for the ascending default
the agent injects, and the warning about Intercom silently
ignoring a sort never fired.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The list of fields `/tickets/search` really filters is not the list
the specification gives: measured during lot 1, it refuses
`company_id` with `invalid_field` although a ticket carries one. So
the first deliverable of the filtering lot is not code, it is the
table of (endpoint x field x operator) the schema will derive its
operators from.

`query/search_fields.yml` holds it, one row per column, each row
carrying its provenance: `measured` for what was observed against a
workspace, `spec` for what was only read off the documentation. The
date operators are measured and differ per endpoint. Everything
else is a candidate until the probe runs.

`Query::SearchFields` reads and validates it. An unknown operator, a
type nothing knows how to send, a column filed as both filterable
and refused: the file ships with the gem and is written against a
script's output, so it fails at boot rather than producing a schema
nobody can explain.

`bin/probe_search_fields` is what measures it, against the
customer's workspace: one search per cell, reading Intercom's
refusal codes, and skipping the rest of a row whose field comes back
`invalid_field`. It writes evidence rather than rewriting the table,
which carries the prose saying why a column stays refused.

The refused tables also record the R7 arbitration: a ticket
attribute is filtered through an id that differs per ticket type, so
the union column cannot say which id to use and the attributes stay
display-only until the customer says otherwise.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The query layer of the filtering lot, derived from the measured
table and from nothing else. Nothing is wired into a collection
yet: publishing a filter before the read can honour it is the
failure this lot exists to prevent, so the schema follows in the
commit that switches the reads to the search endpoints.

`OperatorTable` spells a Forest operator the way the search DSL
does, per kind of field, and narrows it by what the table says the
endpoint accepts. Two rules are worth stating out loud. A date field
carries the two bounds alone, even where the endpoint accepts six:
declaring an equality on a Date column makes the toolkit republish
`in`, which its own validator then refuses (PRD-989), and the
twenty date operators an operator actually uses are all rewritten
into a pair of bounds before they reach here. And `not_i_contains`
is left out although Intercom would answer it, `Rules` not allowing
it on a String column.

A spec asserts the invariant behind both: for every column of every
endpoint, everything the agent publishes from what the column
declares is an operator its own validator allows. That is the check
PRD-989 says nothing currently makes.

`FilterValue` converts what a condition carries into what Intercom
reads: epoch seconds for a date, a bare day being midnight in the
timezone of the caller; the integer a whole float came from; a real
boolean. It refuses the rest by name -- an unparseable date, a cast
that overflowed, an empty list, a list holding a blank, and every
present / blank / missing condition, which the agent rewrites into a
comparison against an empty value that Intercom would answer as if
it were a value of its own.

`ConditionTreeTranslator` walks the tree, unwrapping a branch that
carries a single condition rather than spending one of the two
nesting levels Intercom allows on it. A column the endpoint does not
filter is refused with the reason the table records, a relation is
refused by name, and an operator the field does not answer is
refused with the list of the ones it does.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Intercom truncates a date search to the day, and at the UTC
boundary rather than the workspace's, whatever its documentation
promises. `> V` answers from the start of the day after V; `< V`
answers before the start of V's own day.

Sent as they come, the two bounds the toolkit rewrites an interval
into cancel each other out. `today` reaches the datasource as
`> 00:00` and `< 23:59` of one day, which Intercom reads as "from
tomorrow" and "before today": no rows, to the most ordinary filter
there is, and nothing in the answer saying why.

`DayBounds` moves each bound to the boundary that makes Intercom
answer the day the filter named. A lower bound goes back a day, an
upper bound forward a day -- unless it already sits on a boundary,
where the day it names is exactly the one to leave out. A caller in
UTC therefore gets that day and nothing else; a caller in another
timezone gets the UTC days their window overlaps, up to a day wider
at each end, and is told so once per filter rather than once per
bound.

The window is day-granular either way. That is the granularity the
Intercom interface filters on, and the README section of this lot
will say it plainly rather than let a column imply otherwise.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Intercom nests a search two levels deep and takes fifteen
conditions per group. Past either, it answers a 400 whose body
names neither the limit nor the part of the filter that reached it
-- and the operator reading it has no way of knowing that their
scope, plus their segment, plus their own filter is what went over.

Both are checked before the request leaves, and both refusals name
what to simplify rather than a number alone. A group inside a group
inside a group is one level too many. A group of sixteen says that
a condition naming several values arrives here expanded into one
condition per value, Intercom accepting no membership operator on
these fields, so shortening the list is often what brings it back
under.

The branches the agent wraps around a single condition still spend
no level: it assembles a tree one branch at a time, and unwrapping
them is what keeps an ordinary scope-plus-segment-plus-filter
inside two levels.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lot's point of arrival: a condition switches the read from the
listing to the search endpoint, carrying the query the translator
wrote, and every column advertises exactly the filters that
endpoint answers on it.

The schema derives from the measured table and from nothing else. A
column the table does not carry advertises none, which is how a
refusal is spelled in a schema: the tag names, the account of a
ticket, the columns derived from its parts, and every ticket
attribute stay display-only, and now do so by construction rather
than by a hand-written empty list.

`CursorCollection` grew a fourth route rather than a third: no
condition walks the listing, `id equals X` reads the record
endpoint, and anything else is translated and walked through the
search. Counting follows the same path, `total_count` being exact
on a search too -- a filtered count stays one request over the
whole filtered set rather than over a page of it.

A free-text search reaches Intercom as a condition on the one
column its endpoint matches text on, `source.body`, folded into the
condition tree rather than added to the translated query: written
as one tree, the nesting limits are checked over the whole of it.
Tickets expose no such column and refuse a search by name.

`display_as=plaintext` travels on the search too, in the query
string, where Intercom does not document it. The bodies are HTML
written by end customers (R10), and a filtered read must not come
back as markup where an unfiltered one comes back as text -- if the
endpoint ignores the parameter, that is one of the things the probe
run will settle.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README section of the filtering lot. An operator reads it
before the interface shows them a filter, so it says out loud what
a column can imply but not state: that the table of what each
endpoint filters is measured rather than documented, and how to
measure it again; that a date filter is day-granular and the day is
the UTC one, whatever the workspace timezone says; that the
free-text search matches whole words rather than substrings; that
no column of either collection is sortable; and what stays refused,
one reason per column.

A spec checks the section against the table rather than trusting it
to stay true: the columns the README lists are exactly the ones the
endpoints filter, or the suite fails.

Refs PRD-1118

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 2, 2026

Copy link
Copy Markdown

PRD-1118

@qltysh

qltysh Bot commented Sep 2, 2026

Copy link
Copy Markdown

6 new issues

Tool Category Rule Count
qlty Structure Function with many parameters (count = 6): search_page 4
qlty Structure Function with high complexity (count = 6): print_diff 1
qlty Structure High total complexity (count = 55) 1

end

print_operators(field, column, result[:operators])
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with high complexity (count = 6): print_diff [qlty:function-complexity]

# translator wrote, and it travels in the body; `params` is what still
# belongs in the query string -- `display_as` above all, which is not part
# of the search payload.
def search_page(path, query:, per_page:, starting_after: nil, list_key: 'data', params: {})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with many parameters (count = 6): search_page [qlty:function-parameters]


def post(path, body, boot: false)
(boot ? boot_connection : connection).post(path, body)
def post(path, body, params: {}, boot: false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with many parameters (count = 4): post [qlty:function-parameters]

# while answering a fraction. Refused here rather than through the
# contract's NotImplementedError, which reads as an oversight.
def aggregate(_caller, filter, aggregation, _limit = nil)
def aggregate(caller, filter, aggregation, _limit = nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with many parameters (count = 4): aggregate [qlty:function-parameters]

MAX_DEPTH = 2
MAX_GROUP_SIZE = 15

def self.call(condition_tree, endpoint:, collection:, timezone: nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with many parameters (count = 4): call [qlty:function-parameters]

@qltysh

qltysh Bot commented Sep 2, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (12)

RatingFile% DiffUncovered Line #s
New file Coverage rating: A
...ercom/lib/forest_admin_datasource_intercom/query/day_bounds.rb100.0%
New file Coverage rating: A
...t_admin_datasource_intercom/query/condition_tree_translator.rb100.0%
New file Coverage rating: A
...rcom/lib/forest_admin_datasource_intercom/query/caller_zone.rb100.0%
New file Coverage rating: A
...m/lib/forest_admin_datasource_intercom/query/operator_table.rb100.0%
New file Coverage rating: A
...com/lib/forest_admin_datasource_intercom/query/filter_value.rb100.0%
New file Coverage rating: A
...b/forest_admin_datasource_intercom/collections/conversation.rb100.0%
New file Coverage rating: A
...in_datasource_intercom/lib/forest_admin_datasource_intercom.rb100.0%
New file Coverage rating: A
...est_admin_datasource_intercom/collections/cursor_collection.rb100.0%
New file Coverage rating: A
...om/lib/forest_admin_datasource_intercom/query/search_fields.rb100.0%
New file Coverage rating: A
...source_intercom/lib/forest_admin_datasource_intercom/client.rb100.0%
New file Coverage rating: A
packages/forest_admin_datasource_intercom/bin/probe_search_fields100.0%
New file Coverage rating: A
...com/lib/forest_admin_datasource_intercom/collections/ticket.rb100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

end

def translate_leaf(leaf)
field = @endpoint.field(leaf.field.to_s) || refuse_unfilterable!(leaf.field.to_s)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 High query/condition_tree_translator.rb:104

A compound filter containing id raises UnsupportedOperatorError instead of returning matching records; for example, id = '1' AND state = 'open' is rejected even though id supports equal and in. translate_leaf immediately requires @endpoint.field('id'), while id_lookup only handles a bare primary-key leaf, so the valid id condition is unavailable once nested in a branch. Route id leaves through the primary-key translation path when they appear in compound filters, while preserving their equal/in support.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb around line 104:

A compound filter containing `id` raises `UnsupportedOperatorError` instead of returning matching records; for example, `id = '1' AND state = 'open'` is rejected even though `id` supports `equal` and `in`. `translate_leaf` immediately requires `@endpoint.field('id')`, while `id_lookup` only handles a bare primary-key leaf, so the valid `id` condition is unavailable once nested in a branch. Route `id` leaves through the primary-key translation path when they appear in compound filters, while preserving their `equal`/`in` support.

Comment on lines +55 to +100
conditions = Array(branch.conditions)
refuse_empty_branch!(branch) if conditions.empty?

# Read before the unwrap below, so a branch is refused on the aggregator
# it carries rather than on how many conditions it holds.
operator = aggregator(branch)

# A branch holding one condition needs no group of its own. The agent
# builds a tree one branch at a time -- a scope, then a segment, then the
# operator's own filter -- and the nesting Intercom allows is shallow
# enough that a wrapper around nothing is a level worth not spending.
return translate(conditions.first, depth) if conditions.size == 1

refuse_too_deep!(depth) if depth > MAX_DEPTH
refuse_too_wide!(branch, conditions.size) if conditions.size > MAX_GROUP_SIZE

{ 'operator' => operator, 'value' => conditions.map { |condition| translate(condition, depth + 1) } }
end

# What reaches this depth is a group inside a group inside a group. The
# message names the shape rather than a number, since the tree an operator
# can act on is the segment and the scope they wrote, not the one the agent
# assembled out of them.
def refuse_too_deep!(depth)
raise UnsupportedOperatorError,
"#{@collection} cannot answer this filter: Intercom nests a search #{MAX_DEPTH} levels deep and this " \
"one reaches #{depth}. A group inside a group inside a group is one level too many -- flatten the " \
'segment, the scope or the filter carrying the innermost one.'
end

# Fifteen is reached without trying: a scope, a segment and a filter add up,
# and a condition naming several values is expanded into one condition per
# value on the way here, Intercom taking no membership operator on these
# fields.
def refuse_too_wide!(branch, size)
raise UnsupportedOperatorError,
"#{@collection} cannot answer this filter: Intercom takes #{MAX_GROUP_SIZE} conditions per group and " \
"this #{branch.aggregator} carries #{size}. A filter naming several values counts one condition per " \
'value here, so narrowing the list, the segment or the scope is what brings it back under the limit.'
end

def aggregator(branch)
AGGREGATORS[branch.aggregator.to_s.downcase] ||
raise(UnsupportedOperatorError,
"#{@collection} cannot read #{branch.aggregator.inspect} as a condition tree aggregator; " \
"expected 'And' or 'Or'.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium query/condition_tree_translator.rb:55

An empty Or branch raises UnsupportedOperatorError instead of translating to Intercom's empty-OR query, so ConditionTreeFactory.match_none scopes make valid lists and counts fail rather than return zero records. translate_branch calls refuse_empty_branch! before checking the aggregator; handle empty Or branches as { 'operator' => 'OR', 'value' => [] } while retaining rejection for unsupported empty branches.

         conditions = Array(branch.conditions)
-        refuse_empty_branch!(branch) if conditions.empty?
+        operator = aggregator(branch)
+        return { 'operator' => operator, 'value' => [] } if conditions.empty? && operator == 'OR'
+        refuse_empty_branch!(branch) if conditions.empty?
 
         # Read before the unwrap below, so a branch is refused on the aggregator
@@
-        operator = aggregator(branch)
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb around lines 55-100:

An empty `Or` branch raises `UnsupportedOperatorError` instead of translating to Intercom's empty-OR query, so `ConditionTreeFactory.match_none` scopes make valid lists and counts fail rather than return zero records. `translate_branch` calls `refuse_empty_branch!` before checking the aggregator; handle empty `Or` branches as `{ 'operator' => 'OR', 'value' => [] }` while retaining rejection for unsupported empty branches.

end

def parse(value, leaf)
return start_of_day(Date.parse(value)) if DATE_ONLY.match?(value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 High query/filter_value.rb:78

Invalid shape-matching dates such as "2026-02-30" are normalized by Date.parse and sent as a filter for a different calendar day, so malformed date filters can return incorrect records instead of raising UnsupportedOperatorError. Use strict date parsing for the DATE_ONLY branch so impossible calendar dates raise and are handled by the existing rescue.

Suggested change
return start_of_day(Date.parse(value)) if DATE_ONLY.match?(value)
return start_of_day(Date.strptime(value, '%Y-%m-%d')) if DATE_ONLY.match?(value)
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb around line 78:

Invalid shape-matching dates such as `"2026-02-30"` are normalized by `Date.parse` and sent as a filter for a different calendar day, so malformed date filters can return incorrect records instead of raising `UnsupportedOperatorError`. Use strict date parsing for the `DATE_ONLY` branch so impossible calendar dates raise and are handled by the existing rescue.

# appear, so the row ends here rather than paying a request per shape.
return outcome if outcome[:ok] || outcome[:code] == 'invalid_field'

last = outcome

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium bin/probe_search_fields:93

Transient, authentication, and other non-validation APIErrors are recorded as refused operators, so print_diff and --out report filters as unsupported even though Intercom never evaluated them. probe should treat only field/value validation codes as refusals and propagate unexpected statuses such as 429, 500, and 401 instead of continuing through the value shapes.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_intercom/bin/probe_search_fields around line 93:

Transient, authentication, and other non-validation `APIError`s are recorded as refused operators, so `print_diff` and `--out` report filters as unsupported even though Intercom never evaluated them. `probe` should treat only field/value validation codes as refusals and propagate unexpected statuses such as `429`, `500`, and `401` instead of continuing through the value shapes.

The key was the one column whose Forest operators were not derived
from the table: add_column writes `equal` and `in` on it by hand, the
toolkit refusing a collection whose key carries neither. The table had
no row for it, so the schema advertised a filter the translator had
nothing to write.

A bare `id equals X` never noticed, reading the record endpoint before
the translator is reached. Nested in an `and` it did: a permission
scope turns every record detail into `id equals X and <the scope>`,
which the record endpoint cannot answer -- the ids name a wider set
than the scope does -- so it went to the search and was refused by
name. A scope on either cursor collection therefore broke the record
detail, the count and the CSV export, and a filter on the key from the
interface broke as soon as anything was filtered next to it.

The row goes into the table as `spec`: Intercom documents `id` on both
search endpoints, and the probe is what turns that into a promise. It
comes out of the candidates in the same move.

The invariant spec could not have caught this. It walks the table, and
the gap was a column no row of the table is derived from; walked from
the schema instead, it names the offending column and the endpoint
that cannot translate it. Verified failing without the row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`epoch` read a Numeric with `to_i`, which raises FloatDomainError on an
Infinity or a NaN -- a cast that overflowed above this datasource. The
read then failed with an error naming a float, from a backtrace inside
the value formatter, where every other unusable value comes back as the
refusal naming the column and what the field expects.

The number branch already refuses both, and says why three lines below.
A date is no different; only the guard was missing.

`Integer()` rather than a finite check: it refuses a Complex too, by
the same RangeError, and FloatDomainError is that class one level down.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`FilterFactory.get_previous_condition_tree` writes the bounds of a
previous period with `strftime('%Y-%m-%d %H:%M:%S')`, which carries no
offset at all. `Time.parse` read them in whatever timezone the process
happened to run in, never the caller's -- and because those bounds sit
on a midnight, any offset at all moves them onto another UTC day once
DayBounds truncates. Measured for a Europe/Paris caller asking for the
previous month: the filter names 31 July, Intercom is asked from 1
August. A whole day of rows beside the ones the chart named, on a UTC
server and on a New York one alike.

Reachable through the chart route, which builds the previous-period
filter and counts through `aggregate` -- a bare Count with no group is
exactly what these collections do answer.

`Time.parse` stays, and only for what it refuses: it raises on a string
naming no date, where `Time.zone.parse` answers today. A filter on
`last tuesday` coming back as a filter on today is the silent wrong
answer this datasource exists not to give.

A value carrying an offset is untouched, which is every operator the
toolkit rewrites into a pair of bounds: those arrive as UTC ISO8601 and
read the same in any zone.

The timezone moves out to CallerZone, for the reason DayBounds was split
off already: FilterValue knows what shape a field expects on the wire,
and whether a value carries the caller's wall clock is another question.
The UTC fallback for an unknown timezone now really is UTC on both
paths; on the timestamp one it had been the process timezone, which is
not what its own warning promised.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@christophebrun-forest
christophebrun-forest merged commit 5a00d71 into feat/datasource-intercom Sep 3, 2026
33 checks passed
@christophebrun-forest
christophebrun-forest deleted the feat/prd-1118-search-filters branch September 3, 2026 16:03
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.

1 participant