Throw on mismatched filters in EntityFilter.merge and cover merged-query row distribution

[DZakh]

Hardening follow-up to #1316, addressing review findings.

Throw on mismatched filters in EntityFilter.merge

Previously a filter that didn't match the batch's operator was silently dropped from the merged In query. Since loadByFilter registers an in-memory index for every filter in the batch before querying, a dropped filter would resolve from an index that was never backfilled from the db — silently missing rows.

This is reachable through an operation key collision: toOperationKey prints a single-child And({filters: [Eq({fieldName: "a"})]}) identically to a flat Eq on a (User.getWhere({a: $1})), so the two can land in the same batch. Nothing constructs such a filter today, but before the merge optimization a key collision was harmless — now it would corrupt results, so merge throws loudly instead.

Test coverage for merged-query row distribution

The storage mock previously always returned [], so no test proved that rows returned by a merged In query are routed to the correct per-filter index. MockIndexer.Storage.make now accepts ~dbEntities and serves loadOrThrow by filtering them with EntityFilter.matches, behaving like a real db. New test asserts two concurrent getWhere calls issue a single merged query and each resolves with only its own rows.

https://claude.ai/code/session_01RPHfND7Ge1qS598LWPvfif

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Prevents silent loss of rows when batching incompatible filters and now surfaces clear errors with meaningful operation/parameter context.
• Tests
  • Added tests covering parameter placeholder behavior, error reporting for conflicting batch filters, correct result distribution for merged queries, and improved test storage support for realistic data.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds EntityFilter.getParams and stricter merge validation that throws on incompatible batched filters; updates LoadLayer error context to include operation and params; extends MockIndexer to return filtered dbEntities; adds tests verifying getParams, merge error, and merged-query row distribution.

Changes

Filter merge validation with integration test support

Layer / File(s)	Summary
Filter contract and strict merge behavior packages/envio/src/db/EntityFilter.res	Adds getParams to collect placeholder values and throwUnmergeable; changes Eq→In and In batching to throw JsError on incompatible filters instead of dropping them.
EntityFilter unit tests scenarios/test_codegen/test/EntityFilter_test.res	Adds tests for getParams placeholder flattening and a test asserting merge throws with the exact error message when incompatible filters are batched.
Mock storage infrastructure for filtered entity returns scenarios/test_codegen/test/helpers/MockIndexer.res	Storage.make gains optional ~dbEntities; loadOrThrow returns rows selected from dbEntities filtered by tableName and EntityFilter.matches.
LoadLayer error context and integration test packages/envio/src/LoadLayer.res, scenarios/test_codegen/test/LoadLayer_test.res	loadByFilter now reports { operation: key, params: filter->EntityFilter.getParams } on storage errors. Test verifies two concurrent Eq filters are merged into one In query and returned rows are routed to each original request correctly (with an extra non-matching DB row ignored).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• enviodev/hyperindex#1303: Related changes to the EntityFilter/LoadLayer batching-load path and use of EntityFilter expressions.
• enviodev/hyperindex#1306: Related refactor touching EntityFilter and LoadLayer integration points.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: hardening EntityFilter.merge to throw on mismatched filters and adding test coverage for merged-query row distribution.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/envio/src/LoadLayer.res (1)

400-410: ⚠️ Potential issue | 🟡 Minor

Fix operation/params mismatch in merged Eq→In error logging context

In packages/envio/src/LoadLayer.res the logged operation comes from the outer input filter’s toOperationKey (e.g. Eq => getWhere({field: $1})), but the logged params come from the merged query loop variable’s getParams (e.g. merged In => params is the whole array for _in). So Eq→In batching can produce an operation showing a single $1 while params contains multiple values.

Consider updating the catch block to derive operation from the merged filter (inside the map) or explicitly annotate that params come from an Eq→In merged batch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/src/LoadLayer.res` around lines 400 - 410, The logged
"operation" currently uses the outer variable key while "params" uses the merged
loop variable filter, causing Eq→In mismatch; inside the merged-query catch/map
where you build params (the code referencing filter->EntityFilter.getParams),
change the operation source to the merged filter's operation (use
filter->EntityFilter.toOperationKey or equivalent on the same merged filter) so
both operation and params come from the same filter instance, or alternatively
add an explicit note in the logged message that params are from a merged batch;
update the logging call that sets ~params and ~msg accordingly so operation and
params stay consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/envio/src/LoadLayer.res`:
- Around line 400-410: The logged "operation" currently uses the outer variable
key while "params" uses the merged loop variable filter, causing Eq→In mismatch;
inside the merged-query catch/map where you build params (the code referencing
filter->EntityFilter.getParams), change the operation source to the merged
filter's operation (use filter->EntityFilter.toOperationKey or equivalent on the
same merged filter) so both operation and params come from the same filter
instance, or alternatively add an explicit note in the logged message that
params are from a merged batch; update the logging call that sets ~params and
~msg accordingly so operation and params stay consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6141c829-aa44-4a9e-8d8f-82bbba8318a0

📥 Commits

Reviewing files that changed from the base of the PR and between a706b72 and 9307454.

📒 Files selected for processing (3)

• packages/envio/src/LoadLayer.res
• packages/envio/src/db/EntityFilter.res
• scenarios/test_codegen/test/EntityFilter_test.res

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/envio/src/db/EntityFilter.res