Skip to content

[#7] Added fuzzy matching, highlighting and paging to the choice widgets.#35

Open
AlexSkrypnyk wants to merge 6 commits into
mainfrom
feature/7-fuzzy-matching
Open

[#7] Added fuzzy matching, highlighting and paging to the choice widgets.#35
AlexSkrypnyk wants to merge 6 commits into
mainfrom
feature/7-fuzzy-matching

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Jul 11, 2026

Copy link
Copy Markdown
Member

Closes #7

Summary

This adds fuzzy (subsequence) matching to the suggest, search and multisearch widgets, replacing their case-insensitive substring filter with ranked relevance and matched-character highlighting. A new stateless Matcher finds the best-scoring character embedding for a query within a candidate label, tiering scores so exact, prefix, substring and subsequence matches never cross rank order, and reporting the matched positions so widgets can highlight them. Because ranked results can push long option lists past the viewport, a new ->pageSize() builder method bounds the visible rows across all five choice widgets (select, multiselect, suggest, search, multisearch), paging the list around the cursor with scroll indicators.

Changes

Fuzzy matching core

  • New Matcher (src/Widget/Matcher.php) and MatchResult (src/Widget/MatchResult.php) rank a candidate against a query with tiered scoring, refined by how early and contiguous the match is, via a small O(candidate * needle) dynamic program that selects the highest-scoring character embedding rather than the first (greedy-leftmost) one.
  • The four tiers are modelled as a MatchTier enum (src/Widget/MatchTier.php) - exact, prefix, substring, subsequence - with a weight() ranking multiplier, so the closed set is typed rather than expressed as raw integers.
  • Matching folds each code point on its own rather than the whole string at once, so a matched position always maps back to the original string's offsets even when lowercasing changes a character's length (multibyte-safe).
  • rankValues() and rankOptions() filter and sort candidates by score, keeping equal-score ties in their original declaration order.

Widget integration

  • SuggestWidget, SearchWidget and MultiSearchWidget rank their candidates through the Matcher instead of str_contains(), and report matched positions for highlighting.
  • MultiSelectWidget's checkbox list stays substring-only - fuzzy matching is intentionally scoped to suggest/search/multisearch - but gains filterOptions() and matchPositions() as overridable extension points, which MultiSearchWidget overrides for fuzzy ranking and highlighting.
  • New AbstractWidget::renderMatchedLabel() splits a label into matched/unmatched runs and styles each independently through the theme, so highlighting never nests SGR codes and falls back to the existing highlightLabel() when there are no matched positions.
  • New ThemeInterface::highlightMatch() / DefaultTheme::highlightMatch() theme role for matched characters.

Paging

  • New FieldBuilder::pageSize() / Field::$pageSize (validated positive; NULL falls back to the widget default). It is a visual-only bound and does not constrain a headless value, so it is deliberately excluded from the machine JSON schema.
  • Each choice-widget constructor re-checks the value through a shared AbstractWidget::resolvePageSize(), which throws on a non-positive page size, so direct construction (bypassing the builder) fails loudly instead of rendering a blank window.
  • AbstractWidget::pageViewport() computes a cursor-following window on top of the existing Scroller/Viewport render helpers; SelectWidget, MultiSelectWidget, SearchWidget, SuggestWidget and MultiSearchWidget all page their visible rows through it, rendering up/down scroll indicators when rows are hidden above or below the window.
  • WidgetFactory passes each field's pageSize through to its choice widget's constructor.

Tests and docs

  • MatcherTest covers tier ordering, tightest-embedding selection over greedy-leftmost, multibyte offset correctness, and ranking/tie-break behaviour.
  • Widget tests cover non-contiguous fuzzy matches, prefix-over-subsequence ranking, highlight rendering, the multiselect's substring-only guard, paging (page bounds, scroll indicators, cursor-follow), and the non-positive page-size guard on every choice-widget constructor.
  • README widget sections updated to describe fuzzy ranking, highlighting and ->pageSize().

Before / After

BEFORE - substring filter, no ranking, no paging
┌ Search: git ─────────────────────────┐
│ > GitHub Actions                     │   "gha" typed into the filter matches
│   GitLab CI                          │   nothing - none of the labels contain
│   CircleCI                           │   "gha" as a contiguous substring.
└───────────────────────────────────────┘

┌ Select: option ──────────────────────┐
│ > Option 1                           │
│   Option 2                           │   A long option list overflows the
│   Option 3                           │   viewport - every row renders, however
│   Option 4                           │   many there are.
│   Option 5                           │
│   Option 6                           │
│   Option 7 ...                       │
└───────────────────────────────────────┘

AFTER - ranked fuzzy match with highlighting, bounded and paged
┌ Search: gha ─────────────────────────┐
│ > GitHub Actions                     │   "gha" is a subsequence of "GitHub
│   GitLab CI                          │   Actions" - the G, h and A are
│   CircleCI                           │   highlighted and the label ranks first.
└───────────────────────────────────────┘

┌ Select: option ──────────────────────┐
│  ▲                                   │   Only ->pageSize() rows show at once;
│ > Option 4                           │   the window follows the cursor and the
│   Option 5                           │   up/down indicators mark rows scrolled
│   Option 6                           │   off above and below.
│  ▼                                   │
└───────────────────────────────────────┘

Replaces exact-substring type-to-filter in the 'suggest', 'search' and 'multisearch' widgets with ranked fuzzy (subsequence) matching via a new stateless 'Matcher': matches are scored in tiers (exact, prefix, substring, subsequence) so tighter matches always outrank looser ones, and each match reports the label character indices it hit.

The hit characters are highlighted through a new theme-driven 'highlightMatch' atom, composed run-by-run so it reads on both a plain and a cursor row and strips cleanly in Unicode and ASCII modes.

Adds a configurable '->pageSize()' that bounds the visible option list: the select, multiselect, suggest, search and multisearch widgets now page a cursor-following window with scroll indicators instead of overflowing the editor. The multiselect filter stays substring-only.

Covered by unit tests for ranking, highlight boundaries and paging.
…fsets.

The greedy leftmost subsequence could return a looser embedding than one that existed later in the label, and because the score penalises gaps, the reported positions could score below an alternative the ranking would prefer - so the score and the highlighted characters disagreed. A small O(candidate * needle) dynamic program now returns the embedding the refinement rates highest.

Matching also folded the whole string at once, so a case-fold that changes length (for example 'İ' becoming two code points) desynced the matched indices from the original-string offsets used for highlighting. Folding each code point on its own keeps every position mapped to the original string.
…endering.

The multiselect substring filter now folds case with 'mb_strtolower' so it matches multibyte labels consistently with the fuzzy widgets. The suggest widget's inline match-position lookup moves into a 'positionsFor' helper, mirroring the search widget, and the multi-search paging test now also covers scrolling the window down.
Renamed the 'Matcher' test data providers to match their test method names, wrapped docblock and comment lines at 80 characters, adopted the first-class callable form for the folding map, narrowed the nullable match result with 'instanceof', and asserted a missed match with 'assertNotInstanceOf'.
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ba8b6e06-15fe-4ee4-b2cb-7556b3810b14

📥 Commits

Reviewing files that changed from the base of the PR and between 15d9633 and 8fbb102.

📒 Files selected for processing (2)
  • tests/phpunit/Unit/Widget/SearchWidgetTest.php
  • tests/phpunit/Unit/Widget/SuggestWidgetTest.php
📝 Walkthrough

Walkthrough

Adds fuzzy matching and ranked filtering to Search, Suggest, and MultiSearch, with matched-character highlighting. Select and MultiSelect gain configurable cursor-centered paging, wired through field configuration and documented in the README.

Changes

Choice widget filtering

Layer / File(s) Summary
Page size configuration and wiring
src/Builder/FieldBuilder.php, src/Config/Field.php, src/Widget/WidgetFactory.php, tests/phpunit/Unit/Builder/FormTest.php, README.md
Adds validated pageSize field configuration, passes it into choice widgets, tests invalid and unset values, and documents paging options.
Fuzzy matching and ranking
src/Widget/MatchResult.php, src/Widget/MatchTier.php, src/Widget/Matcher.php, tests/phpunit/Unit/Widget/MatcherTest.php
Adds Unicode-aware fuzzy subsequence matching, relevance scoring, match positions, stable ranking, and coverage for ranking and boundary cases.
Match-aware rendering
src/Theme/ThemeInterface.php, src/Theme/DefaultTheme.php, src/Widget/AbstractWidget.php, tests/phpunit/Unit/Theme/ThemeTest.php
Adds theme styling for matched runs and shared viewport, match-position, and ANSI-safe label rendering helpers.
Fuzzy filtered widgets
src/Widget/SearchWidget.php, src/Widget/SuggestWidget.php, src/Widget/MultiSearchWidget.php, tests/phpunit/Unit/Widget/{SearchWidget,SuggestWidget,MultiSearchWidget}Test.php
Updates filtered widgets to rank fuzzy matches, reset paging after query edits, paginate results, and highlight matched characters.
Select and MultiSelect paging
src/Widget/{SelectWidget,MultiSelectWidget}.php, tests/phpunit/Unit/Widget/{SelectWidget,MultiSelectWidget,WidgetFactory}Test.php
Adds cursor-centered page windows, above/below indicators, and page-size handling while preserving Select and MultiSelect option behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant SearchWidget
  participant Matcher
  participant AbstractWidget
  participant Theme
  User->>SearchWidget: Enter filter query
  SearchWidget->>Matcher: Rank matching options
  Matcher-->>SearchWidget: Ranked options and match positions
  SearchWidget->>AbstractWidget: Compute viewport and render labels
  AbstractWidget->>Theme: Highlight matched character runs
  Theme-->>User: Render paged highlighted results
Loading

Possibly related PRs

  • drevops/tui#21: Modifies the theme interface and atom-based rendering APIs used by match highlighting.

Suggested labels: A4

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.58% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: fuzzy matching, highlighting, and paging for choice widgets.
Linked Issues check ✅ Passed The PR satisfies #7 by adding fuzzy ranking, match highlighting, paging, tests, and README updates for the targeted widgets.
Out of Scope Changes check ✅ Passed The changes stay focused on widget matching, paging, docs, and tests, with no clearly unrelated additions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/7-fuzzy-matching

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

@AlexSkrypnyk AlexSkrypnyk added the A1 Board worker 1 label Jul 11, 2026
@github-actions

This comment has been minimized.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.55%. Comparing base (8cd42c8) to head (8fbb102).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   99.51%   99.55%   +0.03%     
==========================================
  Files          70       73       +3     
  Lines        2082     2239     +157     
==========================================
+ Hits         2072     2229     +157     
  Misses         10       10              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 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/Theme/ThemeInterface.php`:
- Around line 130-133: Adding highlightMatch() to ThemeInterface breaks existing
custom theme implementations; preserve backward compatibility by moving it to a
separate optional interface or reusable trait, and update callers to detect or
safely handle implementations that do not provide it. If retaining the method on
ThemeInterface, explicitly treat this as a major API change and document the
required migration.

In `@src/Widget/AbstractWidget.php`:
- Around line 235-237: Reject non-positive page sizes at the public Field and
SelectWidget/MultiSelectWidget construction boundaries, rather than allowing
them to reach AbstractWidget::pageViewport() and Scroller::compute(). Add
validation requiring pageSize to be greater than zero, preserve existing valid
behavior, and add regression coverage for direct construction with zero or
negative page sizes.

In `@src/Widget/Matcher.php`:
- Line 103: Rename the comparator parameters in both uasort() closures within
the relevant Matcher methods from $a and $b to descriptive snake_case names, and
update the comparison expression accordingly to satisfy PHPMD’s ShortVariable
rule.
- Around line 164-173: Replace the magic integer tiers returned by
Matcher::tier() with a dedicated enum representing exact, prefix, substring, and
subsequence matches; update tier() to return that enum and return its cases
instead of numeric literals. Update any related properties, parameters,
comparisons, sorting, or rendering to use the enum, converting via ->value only
at serialization or display boundaries.

In `@src/Widget/MultiSelectWidget.php`:
- Around line 142-148: Move the shared resetFilterCursor() implementation into
ChoiceListTrait, relying on the consuming widgets’ visible() method, and remove
the duplicate methods from MultiSelectWidget and SearchWidget. Preserve the
existing behavior of assigning cursor via firstSelectable($this->visible()) and
resetting offset to zero.
- Around line 60-63: Validate $pageSize in MultiSelectWidget::__construct so
only positive values are accepted, matching FieldBuilder::pageSize() and
SelectWidget behavior; reject 0 or negative values before assigning the
property, while preserving the default when it is null.

In `@src/Widget/SelectWidget.php`:
- Around line 42-46: Validate the optional $pageSize at the start of
SelectWidget::__construct(), rejecting any value below 1 while allowing NULL to
use DEFAULT_PAGE_SIZE; ensure invalid values cannot reach array_slice().
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 080acf46-b16b-4cee-88ea-45b1c8781c6e

📥 Commits

Reviewing files that changed from the base of the PR and between 8cd42c8 and 23ef844.

📒 Files selected for processing (23)
  • README.md
  • src/Builder/FieldBuilder.php
  • src/Config/Field.php
  • src/Theme/DefaultTheme.php
  • src/Theme/ThemeInterface.php
  • src/Widget/AbstractWidget.php
  • src/Widget/MatchResult.php
  • src/Widget/Matcher.php
  • src/Widget/MultiSearchWidget.php
  • src/Widget/MultiSelectWidget.php
  • src/Widget/SearchWidget.php
  • src/Widget/SelectWidget.php
  • src/Widget/SuggestWidget.php
  • src/Widget/WidgetFactory.php
  • tests/phpunit/Unit/Builder/FormTest.php
  • tests/phpunit/Unit/Theme/ThemeTest.php
  • tests/phpunit/Unit/Widget/MatcherTest.php
  • tests/phpunit/Unit/Widget/MultiSearchWidgetTest.php
  • tests/phpunit/Unit/Widget/MultiSelectWidgetTest.php
  • tests/phpunit/Unit/Widget/SearchWidgetTest.php
  • tests/phpunit/Unit/Widget/SelectWidgetTest.php
  • tests/phpunit/Unit/Widget/SuggestWidgetTest.php
  • tests/phpunit/Unit/Widget/WidgetFactoryTest.php

Comment on lines +130 to +133
/**
* A run of query-matched characters within an option label.
*/
public function highlightMatch(string $text): string;

@coderabbitai coderabbitai Bot Jul 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'src/Theme/*' 'src/**/*.php' 'composer.json' 'README*' 'CHANGELOG*' 'UPGRADE*' 'docs/*' | sed -n '1,200p'

Repository: drevops/tui

Length of output: 5700


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the theme-related files and inspect the interface plus implementations.
ast-grep outline src/Theme/ThemeInterface.php --view expanded || true
ast-grep outline src/Theme/DefaultTheme.php --view expanded || true

Repository: drevops/tui

Length of output: 6579


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the relevant source sections with line numbers.
wc -l src/Theme/ThemeInterface.php src/Theme/DefaultTheme.php 2>/dev/null || true
sed -n '1,240p' src/Theme/ThemeInterface.php
printf '\n----\n'
sed -n '1,260p' src/Theme/DefaultTheme.php

Repository: drevops/tui

Length of output: 13629


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '## Files'
git ls-files 'src/Theme/*' 'src/**/*.php' 'composer.json' 'README*' 'CHANGELOG*' 'UPGRADE*' 'docs/*' | sed -n '1,200p'

echo
echo '## ThemeInterface outline'
ast-grep outline src/Theme/ThemeInterface.php --view expanded || true

echo
echo '## DefaultTheme outline'
ast-grep outline src/Theme/DefaultTheme.php --view expanded || true

echo
echo '## ThemeInterface content'
wc -l src/Theme/ThemeInterface.php
sed -n '1,240p' src/Theme/ThemeInterface.php

echo
echo '## DefaultTheme content'
wc -l src/Theme/DefaultTheme.php
sed -n '1,260p' src/Theme/DefaultTheme.php

Repository: drevops/tui

Length of output: 25702


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '## Implementations of ThemeInterface'
rg -n "implements\s+ThemeInterface|class\s+.*ThemeInterface" src test tests . || true

echo
echo '## References to highlightMatch'
rg -n "highlightMatch\s*\(" src test tests . || true

echo
echo '## Package metadata'
sed -n '1,220p' composer.json

echo
echo '## ThemeInterface'
sed -n '1,220p' src/Theme/ThemeInterface.php

echo
echo '## DefaultTheme'
sed -n '1,220p' src/Theme/DefaultTheme.php

Repository: drevops/tui

Length of output: 16851


Keep ThemeInterface backward compatible
Adding highlightMatch() here breaks any custom theme implementation. Make it optional via a separate interface/trait, or treat this as a major-version API break and document the migration.

🤖 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/Theme/ThemeInterface.php` around lines 130 - 133, Adding highlightMatch()
to ThemeInterface breaks existing custom theme implementations; preserve
backward compatibility by moving it to a separate optional interface or reusable
trait, and update callers to detect or safely handle implementations that do not
provide it. If retaining the method on ThemeInterface, explicitly treat this as
a major API change and document the required migration.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Keeping highlightMatch() on ThemeInterface.

The documented and only supported way to build a theme is to extend DefaultTheme (see its class docblock), which implements highlightMatch(), so consumer themes are unaffected. ThemeInterface deliberately declares every themeable atom as a required method - title, value, marker, radio, check, caret, heading, divider and ~40 others - and each widget feature has added its atom here as the library has grown; highlightMatch() follows that same established pattern rather than being a special case.

The package is pre-1.0 with no published stable-API contract yet, so there is no compatibility guarantee to break. Splitting a single atom into an optional secondary interface (forcing every call site to instanceof-check the theme) would be inconsistent with the other atoms and add complexity for no practical gain given the extend-DefaultTheme model.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@AlexSkrypnyk Thanks for the clarification — that makes sense. Given the pre-1.0 status, the documented "extend DefaultTheme" convention, and the consistency with how every other themeable atom is declared as a required method on ThemeInterface, adding highlightMatch() here is the right call. Withdrawing this concern.

Comment thread src/Widget/AbstractWidget.php
Comment thread src/Widget/Matcher.php Outdated
Comment thread src/Widget/Matcher.php Outdated
Comment thread src/Widget/MultiSelectWidget.php Outdated
Comment thread src/Widget/MultiSelectWidget.php
Comment thread src/Widget/SelectWidget.php Outdated
…zes.

Introduced a 'MatchTier' enum (exact, prefix, substring, subsequence) with a 'weight()' ranking multiplier, so the closed set of tiers is typed rather than raw integers.

Guarded the choice-widget constructors against a non-positive page size with a shared 'resolvePageSize()' that throws before an empty paging window can render, matching the builder's own check, and covered it with tests.

Renamed the sort comparator parameters for clarity.
@github-actions

This comment has been minimized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
src/Widget/SearchWidget.php (1)

43-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a non-positive pageSize test for SearchWidget
SearchWidgetTest.php only covers positive pageSize: 2 cases. Add an exception test for pageSize: 0 or a negative value to lock in the constructor validation.

🤖 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/Widget/SearchWidget.php` around lines 43 - 51, Extend
SearchWidgetTest.php with a constructor test that passes a non-positive
pageSize, such as 0 or a negative value, to SearchWidget::__construct() and
asserts the expected exception is thrown, covering the validation performed by
resolvePageSize().
🤖 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/Widget/AbstractWidget.php`:
- Around line 224-245: Add a use import for InvalidArgumentException in
AbstractWidget.php, then update resolvePageSize() to reference the imported
class instead of the fully qualified \InvalidArgumentException.

In `@src/Widget/SuggestWidget.php`:
- Around line 38-41: Add a RejectsNonPositivePageSize test in
SuggestWidgetTest.php that constructs SuggestWidget with zero and negative
pageSize values and asserts both are rejected by the shared resolvePageSize()
guard, matching existing page-size validation test conventions.

---

Outside diff comments:
In `@src/Widget/SearchWidget.php`:
- Around line 43-51: Extend SearchWidgetTest.php with a constructor test that
passes a non-positive pageSize, such as 0 or a negative value, to
SearchWidget::__construct() and asserts the expected exception is thrown,
covering the validation performed by resolvePageSize().
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3a5b2415-29f5-4e3d-9e38-e478543e7dd2

📥 Commits

Reviewing files that changed from the base of the PR and between 23ef844 and 15d9633.

📒 Files selected for processing (10)
  • src/Widget/AbstractWidget.php
  • src/Widget/MatchTier.php
  • src/Widget/Matcher.php
  • src/Widget/MultiSelectWidget.php
  • src/Widget/SearchWidget.php
  • src/Widget/SelectWidget.php
  • src/Widget/SuggestWidget.php
  • tests/phpunit/Unit/Widget/MatcherTest.php
  • tests/phpunit/Unit/Widget/MultiSelectWidgetTest.php
  • tests/phpunit/Unit/Widget/SelectWidgetTest.php

Comment on lines +224 to +245
/**
* Resolve the effective page size, rejecting a non-positive declared value.
*
* The builder rejects a non-positive page size, but a widget may be
* constructed directly, so the invariant is enforced here too.
*
* @param int|null $pageSize
* The declared page size, or NULL to use the default.
*
* @return int
* The effective page size.
*
* @throws \InvalidArgumentException
* When a declared page size is not positive.
*/
protected function resolvePageSize(?int $pageSize): int {
if ($pageSize !== NULL && $pageSize < 1) {
throw new \InvalidArgumentException(sprintf('Page size must be a positive integer, %d given.', $pageSize));
}

return $pageSize ?? self::DEFAULT_PAGE_SIZE;
}

@coderabbitai coderabbitai Bot Jul 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a use import for InvalidArgumentException.

Static analysis flags the inline fully-qualified \InvalidArgumentException at line 241 for a missing import.

🧹 Proposed fix
+use InvalidArgumentException;
+
 abstract class AbstractWidget implements WidgetInterface {
-      throw new \InvalidArgumentException(sprintf('Page size must be a positive integer, %d given.', $pageSize));
+      throw new InvalidArgumentException(sprintf('Page size must be a positive integer, %d given.', $pageSize));
🧰 Tools
🪛 PHPMD (2.15.0)

[error] 241-241: Missing class import via use statement (line '241', column '17'). (undefined)

(MissingImport)

🤖 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/Widget/AbstractWidget.php` around lines 224 - 245, Add a use import for
InvalidArgumentException in AbstractWidget.php, then update resolvePageSize() to
reference the imported class instead of the fully qualified
\InvalidArgumentException.

Source: Linters/SAST tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Keeping the fully-qualified \InvalidArgumentException.

This matches the existing convention in the codebase - DefaultTheme::validateOptions() throws \InvalidArgumentException the same way, with no use import, and passes phpcs and CI. The project's lint is phpcs + PHPStan + Rector (PHPMD is not part of it), and phpcs is satisfied with the fully-qualified global-namespace reference. Adding a use import here would make AbstractWidget inconsistent with the rest of the code.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'InvalidArgumentException' src/Theme/DefaultTheme.php src/**/*.php -g '*.php' | head -50

Length of output: 8229


@AlexSkrypnyk Confirmed — the codebase consistently uses the fully-qualified \InvalidArgumentException without a use import across src/Theme/DefaultTheme.php, src/Theme/ThemeManager.php, src/Input/KeyMap.php, and src/Input/KeyMapManager.php. Keeping AbstractWidget.php consistent with this convention makes sense, and since PHPMD isn't part of your lint pipeline (phpcs + PHPStan + Rector), there's no actual conflict. Withdrawing this suggestion.


🧬 Code Graph Analysis Results

Relevant snippet: src/Widget/AbstractWidget.php

resolvePageSize(?int $pageSize): int (throws fully-qualified \InvalidArgumentException) — lines 228-234

protected function resolvePageSize(?int $pageSize): int {
  if ($pageSize !== NULL && $pageSize < 1) {
    throw new \InvalidArgumentException(sprintf('Page size must be a positive integer, %d given.', $pageSize));
  }

  return $pageSize ?? self::DEFAULT_PAGE_SIZE;
}

Relevant snippets that call AbstractWidget::resolvePageSize()

src/Widget/MultiSelectWidget.php

Constructor uses resolvePageSize($pageSize) — lines 60-73

public function __construct(array $options, array $default = [], ?\Closure $validate = NULL, ?\Closure $transform = NULL, ?int $pageSize = NULL) {
    parent::__construct($validate, $transform);
    $this->initOptions($options);
    $this->pageSize = $this->resolvePageSize($pageSize);

    $selectable = array_fill_keys($this->selectableValues(), TRUE);
    foreach ($default as $value) {
      if (isset($selectable[$value])) {
        $this->selected[$value] = TRUE;
      }
    }

    $this->cursor = $this->firstSelectable($this->options);
}

src/Widget/SuggestWidget.php

Constructor uses resolvePageSize($pageSize) — lines 38-41

public function __construct(array $values, string $buffer = '', ?\Closure $validate = NULL, ?\Closure $transform = NULL, ?int $pageSize = NULL) {
    parent::__construct($validate, $transform);
    $this->pageSize = $this->resolvePageSize($pageSize);
}

src/Widget/SearchWidget.php

Constructor uses resolvePageSize($pageSize) — lines 47-52

public function __construct(array $options, string $default = '', ?\Closure $validate = NULL, ?\Closure $transform = NULL, ?int $pageSize = NULL) {
    parent::__construct($validate, $transform);
    $this->initOptions($options);
    $this->cursor = $this->cursorForDefault($this->options, $default);
    $this->pageSize = $this->resolvePageSize($pageSize);
}

src/Widget/SelectWidget.php

Constructor uses resolvePageSize($pageSize) — lines 42-47

public function __construct(array $options, string $default = '', ?\Closure $validate = NULL, ?\Closure $transform = NULL, ?int $pageSize = NULL) {
    parent::__construct($validate, $transform);
    $this->initOptions($options);
    $this->cursor = $this->cursorForDefault($this->options, $default);
    $this->pageSize = $this->resolvePageSize($pageSize);
}

Related snippet showing the same exception style (new \InvalidArgumentException(...))

src/Input/KeyMapManager.php

register() and create() throw fully-qualified \InvalidArgumentException — lines 20-85

public static function register(string $name, string $class): void {
  if (!is_a($class, DefaultKeyMap::class, TRUE)) {
    throw new \InvalidArgumentException(sprintf('Key-map preset class "%s" must extend %s.', $class, DefaultKeyMap::class));
  }

  self::$registry[$name] = $class;
}

/**
 * Create a resolved key map from a preset and optional overrides.
 *
 * Lowest friction first: a fully-qualified preset class name is instantiated
 * directly, so a config can point at a consumer's own preset class with no
 * registration. Otherwise the name is looked up in the registry ("default",
 * "vim" or a name passed to {`@see` register()}). An unknown name fails
 * loudly - a typo should not silently fall back to the defaults.
 *
 * `@param` string $name
 *   A registered name, a preset class name, or "" for the default preset.
 * `@param` list<\DrevOps\Tui\Input\Binding> $overrides
 *   Bindings appended after the preset's own, retuning individual bindings.
 *
 * `@return` \DrevOps\Tui\Input\KeyMap
 *   The resolved, validated key map.
 *
 * `@throws` \InvalidArgumentException
 *   When the name is neither registered nor a preset class name, or a binding
 *   conflicts or is malformed.
 */
public static function create(string $name = 'default', array $overrides = []): KeyMap {
  $name = $name === '' ? 'default' : $name;

  $class = self::$registry[$name] ?? (is_a($name, DefaultKeyMap::class, TRUE) ? $name : NULL);

  if ($class === NULL) {
    throw new \InvalidArgumentException(sprintf('Unknown key-map preset "%s". Use a registered name (%s), register one with KeyMapManager::register(), or pass a DefaultKeyMap subclass name.', $name, implode(', ', array_keys(self::$registry))));
  }

  return new KeyMap(array_merge((new $class())->bindings(), $overrides));
}

Comment thread src/Widget/SuggestWidget.php
Added the page-size guard regression test to the suggest and search widgets, so all four choice-widget constructors verify that a non-positive page size is rejected.
@github-actions

Copy link
Copy Markdown
Code Coverage Report:
  2026-07-11 00:42:51

 Summary:
  Classes: 93.15% (68/73)
  Methods: 98.26% (508/517)
  Lines:   99.55% (2229/2239)

DrevOps\Tui\Answers\Answer
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Answers\Answers
  Methods: 100.00% ( 9/ 9)   Lines: 100.00% ( 20/ 20)
DrevOps\Tui\Answers\SummaryFormatter
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 20/ 20)
DrevOps\Tui\Builder\FieldBuilder
  Methods: 100.00% (30/30)   Lines: 100.00% (101/101)
DrevOps\Tui\Builder\Form
  Methods:  93.33% (14/15)   Lines:  98.44% ( 63/ 64)
DrevOps\Tui\Builder\PanelBuilder
  Methods: 100.00% (19/19)   Lines: 100.00% ( 31/ 31)
DrevOps\Tui\Condition\CompositeCondition
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 19/ 19)
DrevOps\Tui\Condition\Condition
  Methods: 100.00% (10/10)   Lines: 100.00% ( 38/ 38)
DrevOps\Tui\Config\Config
  Methods: 100.00% ( 5/ 5)   Lines: 100.00% ( 17/ 17)
DrevOps\Tui\Config\Field
  Methods: 100.00% ( 5/ 5)   Lines: 100.00% ( 30/ 30)
DrevOps\Tui\Config\FieldType
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% (  2/  2)
DrevOps\Tui\Config\NumberBounds
  Methods: 100.00% ( 6/ 6)   Lines: 100.00% ( 20/ 20)
DrevOps\Tui\Config\Option
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% (  9/  9)
DrevOps\Tui\Config\Panel
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Derive\Derive
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 14/ 14)
DrevOps\Tui\Derive\Deriver
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% ( 13/ 13)
DrevOps\Tui\Derive\Transform
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% (  7/  7)
DrevOps\Tui\Discovery\AbstractDiscover
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Discovery\Dotenv
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 19/ 19)
DrevOps\Tui\Discovery\JsonValue
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 16/ 16)
DrevOps\Tui\Discovery\PathExists
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% (  3/  3)
DrevOps\Tui\Discovery\Scan
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 20/ 20)
DrevOps\Tui\Engine\Engine
  Methods: 100.00% (12/12)   Lines: 100.00% ( 95/ 95)
DrevOps\Tui\Handler\HandlerRegistry
  Methods:  85.71% ( 6/ 7)   Lines:  95.45% ( 21/ 22)
DrevOps\Tui\Input\ArrayKeyStream
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 12/ 12)
DrevOps\Tui\Input\Binding
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Input\DefaultKeyMap
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% ( 33/ 33)
DrevOps\Tui\Input\Key
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% (  7/  7)
DrevOps\Tui\Input\KeyMap
  Methods: 100.00% (12/12)   Lines: 100.00% ( 65/ 65)
DrevOps\Tui\Input\KeyMapManager
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% (  8/  8)
DrevOps\Tui\Input\KeyParser
  Methods: 100.00% ( 6/ 6)   Lines: 100.00% ( 68/ 68)
DrevOps\Tui\Input\Scope
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 11/ 11)
DrevOps\Tui\Input\ScopedKeyMap
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% (  4/  4)
DrevOps\Tui\Input\VimKeyMap
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  8/  8)
DrevOps\Tui\Render\Ansi
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% (  5/  5)
DrevOps\Tui\Render\Box
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 15/ 15)
DrevOps\Tui\Render\ExternalEditor
  Methods: 100.00% ( 6/ 6)   Lines: 100.00% ( 27/ 27)
DrevOps\Tui\Render\Navigator
  Methods: 100.00% ( 6/ 6)   Lines: 100.00% ( 15/ 15)
DrevOps\Tui\Render\PanelController
  Methods: 100.00% (16/16)   Lines: 100.00% ( 95/ 95)
DrevOps\Tui\Render\Scroller
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 12/ 12)
DrevOps\Tui\Render\Terminal
  Methods: 100.00% (12/12)   Lines: 100.00% ( 35/ 35)
DrevOps\Tui\Render\TerminalControl
  Methods: 100.00% ( 9/ 9)   Lines: 100.00% (  9/  9)
DrevOps\Tui\Render\Viewport
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Resolver\InputResolver
  Methods: 100.00% ( 6/ 6)   Lines: 100.00% ( 27/ 27)
DrevOps\Tui\Schema\AgentHelp
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 25/ 25)
DrevOps\Tui\Schema\SchemaGenerator
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 30/ 30)
DrevOps\Tui\Schema\SchemaValidator
  Methods: 100.00% ( 8/ 8)   Lines: 100.00% ( 37/ 37)
DrevOps\Tui\Theme\DefaultTheme
  Methods:  92.54% (62/67)   Lines:  97.27% (214/220)
DrevOps\Tui\Theme\ThemeManager
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% (  8/  8)
DrevOps\Tui\Tui
  Methods: 100.00% (12/12)   Lines: 100.00% ( 26/ 26)
DrevOps\Tui\Widget\AbstractWidget
  Methods: 100.00% (18/18)   Lines: 100.00% ( 48/ 48)
DrevOps\Tui\Widget\ChoiceListTrait
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 21/ 21)
DrevOps\Tui\Widget\ConfirmWidget
  Methods: 100.00% ( 5/ 5)   Lines: 100.00% ( 22/ 22)
DrevOps\Tui\Widget\FilePickerWidget
  Methods: 100.00% (30/30)   Lines: 100.00% (187/187)
DrevOps\Tui\Widget\MatchResult
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
DrevOps\Tui\Widget\MatchTier
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  6/  6)
DrevOps\Tui\Widget\Matcher
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 79/ 79)
DrevOps\Tui\Widget\MultiSearchWidget
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% (  3/  3)
DrevOps\Tui\Widget\MultiSelectWidget
  Methods: 100.00% (14/14)   Lines: 100.00% ( 98/ 98)
DrevOps\Tui\Widget\NumberWidget
  Methods: 100.00% ( 9/ 9)   Lines: 100.00% ( 37/ 37)
DrevOps\Tui\Widget\PasswordDisplay
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  5/  5)
DrevOps\Tui\Widget\PasswordWidget
  Methods: 100.00% ( 8/ 8)   Lines: 100.00% ( 44/ 44)
DrevOps\Tui\Widget\PauseWidget
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 10/ 10)
DrevOps\Tui\Widget\SearchWidget
  Methods: 100.00% ( 8/ 8)   Lines: 100.00% ( 57/ 57)
DrevOps\Tui\Widget\SelectWidget
  Methods: 100.00% ( 5/ 5)   Lines: 100.00% ( 36/ 36)
DrevOps\Tui\Widget\SuggestWidget
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 47/ 47)
DrevOps\Tui\Widget\TextWidget
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 31/ 31)
DrevOps\Tui\Widget\TextareaWidget
  Methods: 100.00% ( 8/ 8)   Lines: 100.00% ( 52/ 52)
DrevOps\Tui\Widget\ToggleWidget
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 31/ 31)
DrevOps\Tui\Widget\WidgetFactory
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 30/ 30)
DrevOps\Tui\Widget\WidgetRunner
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  5/  5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A1 Board worker 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add fuzzy matching to the suggest, search and multisearch widgets

1 participant