feat(filtering): support columnOverrides.read in findRow/findRows/countRows

[rickcedwhat-ai]

Summary

• Columns with a columnOverrides.read function are now respected when filtering via findRow, findRows, and countRows (feat(findRow): support filtering on columnOverrides / synthetic columns #385)
• Filter keys are split into DOM filters (applied via Playwright locator chain) and override filters (post-filtered in Node.js by evaluating the read function against string/number/RegExp values)
• Added splitFilters() and matchesOverrideFilters() to RowFinder, and override-aware counting in countRows

Closes #385

Test plan

• findRow filters on override-produced value (reads <a href> from Link cell)
• findRows filters on override-produced value (substring match)
• findRow combines DOM filter + override filter
• findRow throws Ambiguous Row when override filter matches multiple
• countRows with override filter counts only matching rows
• All 193 existing E2E tests pass
• All 313 unit tests pass
• TypeScript compiles clean

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for filtering rows using values read from column overrides.
  • Override-based filters now support exact matches, partial text matches, and regular expressions.
  • Combined DOM and override filters are supported when finding or counting rows.
  • Row selection now correctly reports when override filters match multiple rows.

• Bug Fixes

  • Improved row counts and matching behavior across paginated and non-paginated tables.

[rickcedwhat-ai]

🤖 Bot HQ

🔗 Issue Link

🔗 Closes #385

---

🔍 AI Review

• 🔍 Request AI review

✅ Review complete — no blocking issues.

📊 This PR: $0.0403 · Repo today: $0.04 / $1.00 · Month: $1.35 / $10.00

Review History (1 round)

Round	Date	Commit	In Tokens	Out Tokens	Cost	Summary
1	2026-07-29	136830f	37,606	538	$0.0403	ColumnOverrideReadContext not passed completely in countRows (3 issues)

---

_{This comment is managed by the bot — do not edit directly.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 38df04a6-8033-4d2e-91b9-4e3a3ad55946

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (2)

src/engine/rowFinder.ts (2)

260-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate "Ambiguous Row" error construction across both branches.

The sample-data collection and error-throwing logic (Lines 260-276 and 290-305) are near-identical, differing only in the source array (matchedRows vs overrideMatches). Consider extracting a private helper, e.g. private async throwIfAmbiguous(matches: Locator[], filters, map): Promise<void>, called from both branches, so the message format can't drift between the DOM-only and override paths.

Also applies to: 290-305

🤖 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 `@src/engine/rowFinder.ts` around lines 260 - 276, Extract the duplicated
ambiguity handling from both branches into a private helper, such as
throwIfAmbiguous, that accepts the matching rows source, filters, and map,
collects sample data, and throws the existing “Ambiguous Row” error when
multiple matches are found. Replace the near-identical logic in the matchedRows
and overrideMatches paths with calls to this helper, preserving the current
message and sample behavior.

---

279-288: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Sequential per-candidate override evaluation.

matchesOverrideFilters is awaited one candidate at a time (Lines 283-287). Since each call may perform a page read via override.read, this serializes what could be parallel round trips. Consider Promise.all(candidates.map(c => this.matchesOverrideFilters(c, overrideFilters, map, options.exact || false))) and filtering the results, preserving candidate order for the sample-data reporting above.

🤖 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 `@src/engine/rowFinder.ts` around lines 279 - 288, Update the override-filter
evaluation in the candidate loop to invoke matchesOverrideFilters for all
candidates concurrently via Promise.all, then retain only candidates whose
results are true. Preserve the original candidate order when constructing
overrideMatches and keep the surrounding logging unchanged.

🤖 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.

Inline comments:
In `@src/engine/rowFinder.ts`:
- Around line 44-76: The override-filter matching logic must handle
function-typed FilterValue instead of silently treating it as a match. Update
matchesOverrideFilters in src/engine/rowFinder.ts (lines 44-76) and
countOverrideMatches in src/useTable.ts (lines 357-387) to apply function
filters to the resolved cell or throw a clear unsupported-filter error;
preferably centralize the shared value-matching logic so both paths remain
consistent.

In `@src/useTable.ts`:
- Around line 357-387: Update countOverrideMatches to validate the result of
tableMapper.getMapSync() before using it, matching the descriptive
initialization-error guard already used by resolveRows. Remove the bare non-null
assertion and throw the same established error when the map is unavailable.
- Around line 389-395: Update the no-pagination override-filter branch in the
counting flow to reuse the candidates returned by the existing rows.all() call
when invoking countOverrideMatches, rather than allowing it to query the locator
again. Adjust countOverrideMatches and its callers as needed so the same fetched
row set supplies both indices and matching, preserving the existing count
behavior without a second DOM query.

In `@tests/override-filter.spec.ts`:
- Around line 40-46: Update the findRows test to query the Link override with
the selective value `/d/beta` and `{ exact: true }`, then assert exactly one
result is returned and that it is the Beta row, ensuring override filtering is
actually applied.
- Around line 16-29: Replace the any parameter in makeTable with Playwright’s
Page type by importing type Page from `@playwright/test` and annotating the page
argument as Page. Keep the existing useTable configuration and Link read
behavior unchanged.

---

Nitpick comments:
In `@src/engine/rowFinder.ts`:
- Around line 260-276: Extract the duplicated ambiguity handling from both
branches into a private helper, such as throwIfAmbiguous, that accepts the
matching rows source, filters, and map, collects sample data, and throws the
existing “Ambiguous Row” error when multiple matches are found. Replace the
near-identical logic in the matchedRows and overrideMatches paths with calls to
this helper, preserving the current message and sample behavior.
- Around line 279-288: Update the override-filter evaluation in the candidate
loop to invoke matchesOverrideFilters for all candidates concurrently via
Promise.all, then retain only candidates whose results are true. Preserve the
original candidate order when constructing overrideMatches and keep the
surrounding logging unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b27aea8b-1688-467b-b5d7-f89f92c0944a

📥 Commits

Reviewing files that changed from the base of the PR and between d9acaf6 and d41c916.

📒 Files selected for processing (3)

• src/engine/rowFinder.ts
• src/useTable.ts
• tests/override-filter.spec.ts

[rickcedwhat-ai]

AI Review

PR #398 adds support for filtering on columnOverrides.read values in findRow, findRows, and countRows. Implementation splits filters into DOM and override filters, applies overrides post-DOM-match for accurate filtering. All 193 E2E tests pass. Found 3 issues: incomplete ColumnOverrideReadContext type usage, missing null check for column name resolution, and potential barrier leak in override filter path.

📋 3 issues — expand to copy prompt

You are an AI code reviewer resolving findings from a pull request review.

For each issue below:
1. Determine whether the finding is valid.
2. If valid — fix the code, commit your changes to the branch, then record [N] fix.
3. If not valid (false positive or intentional design) — record [N] skip - <your reason>.

After addressing all issues, post a single comment to this PR with one line per issue:
  [N] fix
  [N] skip - reason

Do not reply to the user who shared this prompt — reply directly in the PR.
---

[1] ColumnOverrideReadContext not passed completely in countRows — src/useTable.ts:374
The `ctx` object passed to `override.read()` is missing the `columnIndex` field from ColumnOverrideReadContext interface. The interface expects `columnIndex: number` but the code only provides `row`, `columnName`, and `getCell`. Add `columnIndex: map.get(colName)!` to match the type signature.

[2] Unsafe column index lookup without null check in countRows — src/useTable.ts:365
The code assumes `map.get(colName)` returns a number but doesn't validate it. Although there's a continue after the check on line 366, the subsequent line 369 uses colIndex without confirming it's defined, which could cause a type error. Add explicit null guard or restructure the logic to ensure colIndex is never used when undefined.

[3] Navigation barrier not marked finished on early continue in findRows — src/engine/rowFinder.ts:180
When override filters don't match, the code continues without calling `barrier?.markFinished()`. This leaves the barrier waiting for a signal that never comes, potentially deadlocking other rows in synchronized concurrency mode. Call `barrier?.markFinished()` before the continue statement on line 181.

⚙️ [1] ColumnOverrideReadContext not passed completely in countRows — src/useTable.ts:374

The ctx object passed to override.read() is missing the columnIndex field from ColumnOverrideReadContext interface. The interface expects columnIndex: number but the code only provides row, columnName, and getCell. Add columnIndex: map.get(colName)! to match the type signature.

⚙️ [2] Unsafe column index lookup without null check in countRows — src/useTable.ts:365

The code assumes map.get(colName) returns a number but doesn't validate it. Although there's a continue after the check on line 366, the subsequent line 369 uses colIndex without confirming it's defined, which could cause a type error. Add explicit null guard or restructure the logic to ensure colIndex is never used when undefined.

⚙️ [3] Navigation barrier not marked finished on early continue in findRows — src/engine/rowFinder.ts:180

When override filters don't match, the code continues without calling barrier?.markFinished(). This leaves the barrier waiting for a signal that never comes, potentially deadlocking other rows in synchronized concurrency mode. Call barrier?.markFinished() before the continue statement on line 181.

[rickcedwhat]

@rickcedwhat-ai override all

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-29 17:55 UTC