feat(ui): persist admin table filters across HTMX pagination and partial refresh by omorros · Pull Request #3647 · IBM/mcp-context-forge

omorros · 2026-03-12T11:00:29Z

🔗 Related Issue

Closes #2992 Replaces #3204 (could not push conflict resolution to upstream branch). Original work from #3017 and #3100.

📝 Summary

Persist admin table filters (search text, tag filters, "show inactive" toggles) across HTMX pagination clicks and
partial refreshes for all 6 admin tables.

Root cause: pagination_controls.html loadPage() built HTMX request URLs using template-baked query_params (static Jinja2 values). When pagination controls were rendered before a search, clicking page 2 dropped the active
filters.

Changes:

pagination_controls.html — loadPage() now reads live namespaced q/tags from the browser URL (source of truth
written by updatePanelSearchStateInUrl), overriding stale template-baked values.
admin.js — Added htmx:afterSettle listener that rehydrates search inputs from URL state after content swaps.
Added updateFilterStatus() for visible filtered row count text. - admin.html — Added aria-live="polite" status spans next to all 6 table search bars (servers, tools, resources,
prompts, gateways, a2a-agents).

🏷️ Type of Change

🧪 Verification

Check	Command	Status
JS tests (vitest)	`npx vitest run`	583/584 pass (1 pre-existing locale mismatch)
Unit tests	`make test`	12,349/12,355 pass (6 pre-existing stdio failures)
Lint suite	`make lint`	N/A — no Python files changed
Coverage ≥ 80%	`make coverage`	N/A — no Python files changed

✅ Checklist

Code formatted (make black isort pre-commit)
Tests added/updated for changes
Documentation updated (if applicable)
No secrets or credentials committed

📓 Notes (optional)

Rebased on latest main to resolve merge conflicts from #3204. The code and functionality are identical to the
previously approved PR.

marekdano

Thanks for the PR, @omorros!
This PR correctly fixes the filter persistence issue across pagination with a clean, well-architected solution.

🎯 Key Strengths

Root Cause Correctly Identified: The issue was that loadPage() in pagination_controls.html used template-baked query_params (static Jinja2 values rendered at page load), which became stale after user interactions.
Elegant Solution: The fix establishes a clear data flow:

Source of Truth: Browser URL (written by updatePanelSearchStateInUrl())
Pagination: Reads live state from URL via currentUrlParams.get(livePrefix + 'q')
Rehydration: htmx:afterSettle listener restores input values after HTMX swaps

Defense-in-Depth: The fix includes multiple layers:

Primary: Read live URL params in loadPage() (lines 139-151 in pagination_controls.html)
Fallback: Still reads from input elements (lines 115-137) for backward compatibility
Rehydration: Restores inputs after HTMX swaps (lines 18743-18760 in admin.js)

User Feedback: Added updateFilterStatus() with aria-live regions for accessibility

📋 Technical Review

pagination_controls.html (+13 lines):

✅ Correctly reads namespaced params (e.g., servers_q, tools_tags)
✅ Overrides stale template values
✅ Preserves team_id for multi-tenancy

admin.js (+65 lines):

updateFilterStatus() (lines 18703-18738):

✅ Reads URL params to detect active filters
✅ Displays filtered count from pagination controls
✅ Falls back to "Filters active" if count unavailable
✅ Clears status when no filters active

htmx:afterSettle listener (lines 18743-18760):

✅ Triggers on table/pagination swaps
✅ Calls resetSearchInputsState() + initializeSearchInputsMemoized()
✅ Updates filter status after rehydration

admin.html (+6 lines):

✅ Added to all 6 tables
✅ Proper aria-live="polite" and role="status" for accessibility

🔍 Minor Observations

Redundant Input Reading: Lines 115-137 in pagination_controls.html still read from input elements before the URL override. This is harmless (URL wins) but could be simplified in a future refactor.
Event Listener Scope: The htmx:afterSettle listener triggers on any element ending with -table, -table-body, -list-container, or -pagination-controls. This is broad but safe since initializeSearchInputsMemoized() is memoized.
Filter Status Timing: updateFilterStatus() reads pagination totals from Alpine-rendered elements. If Alpine hasn't rendered yet, it falls back to "Filters active" (acceptable).

✅ Verification Checklist

✅ Fixes root cause (stale template-baked params)
✅ Maintains backward compatibility (still reads inputs as fallback)
✅ Handles all 6 admin tables (servers, tools, resources, prompts, gateways, a2a-agents)
✅ Preserves team_id for multi-tenancy
✅ Accessibility (aria-live regions)
✅ Clean, minimal changes (84 additions, 0 deletions)
✅ No breaking changes

This PR correctly fixes the filter persistence bug with a well-architected solution. The redundant input reading is harmless and can be cleaned up in a future refactor if desired.

LGTM 🚀

crivetimihai · 2026-03-17T22:41:00Z

Maintainer Review

Rebased onto latest main and resolved a merge conflict in pagination_controls.html.

Conflict Resolution

Main recently added DOM-based search input reading in loadPage() (fix for #3128), which conflicts with this PR's URL-based approach. Kept the URL-based approach because:

The DOM approach had an ID mismatch bug: it used tableName + '-search-input' (e.g., servers-search-input) but the catalog panel's actual input ID is catalog-search-input. This means search filters were silently dropped during pagination for at least the servers/catalog tab.
URL is the source of truth: updatePanelSearchStateInUrl() is always called before HTMX requests, so reading from URL params is both simpler and more reliable.

Test Coverage Added

Added 34 unit tests in admin-pagination.test.js covering all new code:

updateFilterStatus() — 9 tests: clear/active filter states, pagination text integration, per-table independence, no-op safety for missing elements
htmx:afterSettle rehydration — 7 tests: all target suffix patterns (-table, -table-body, -list-container, -pagination-controls), unrelated targets, null/empty guards
URL-param filter reading — 12 tests: namespaced q/tags reading, extraParams override precedence, team_id preservation, namespace isolation between tables, empty value handling

Full test suite: 899/899 JS tests pass, 0 regressions.

Review Notes

No security concerns — uses textContent (not innerHTML), no user input rendered as HTML
No performance concerns — iterates 6-item config, no heavy operations
Accessibility: aria-live="polite" and role="status" on filter-status spans is correct usage
Consistent with existing patterns (PANEL_SEARCH_CONFIG, createMemoizedInit, namespaced URL params)

crivetimihai

LGTM — rebased, reviewed, and hardened.

Changes made on top of the original commit:

Conflict resolution — kept URL-based filter reading (correct), dropped main's DOM-based approach (had catalog/a2a-agents input ID mismatch bug)
Debounce race fix — added flushPendingSearchState() so paginating within the 250ms debounce window still picks up live input values via PANEL_SEARCH_CONFIG (correct IDs for all 6 tables)
40 new tests — updateFilterStatus, htmx:afterSettle rehydration, URL-param filter reading, and flush function coverage

905/905 JS tests pass, all linters clean.

crivetimihai · 2026-03-17T23:52:51Z

Additional fix: DOM fallback for restricted iframe contexts

On review, the URL-only approach would regress filter persistence in embedded/iframe deployments where safeReplaceState() is a no-op (the URL never gets updated, so reading from it returns nothing).

Fix: Added getLiveSearchState(tableName) which reads DOM inputs via PANEL_SEARCH_CONFIG — this uses the correct input IDs for all 6 tables (including catalog-search-input for servers and a2a-agents-search-input for agents). loadPage() now uses a layered strategy:

Flush any pending debounce timers to URL (flushPendingSearchState)
Read from URL (primary — works in normal contexts)
DOM fallback (getLiveSearchState) — kicks in when URL params are empty, covering restricted iframe contexts

14 additional tests covering getLiveSearchState (correct ID mapping for all tables including catalog/a2a-agents edge cases) and the DOM fallback logic.

919/919 JS tests pass (1 pre-existing flaky gantt-chart test occasionally fails in parallel runs, passes in isolation).

crivetimihai · 2026-03-18T00:07:25Z

Fix: DOM-first precedence for filter reads

The previous layering was wrong — URL params took precedence over DOM inputs, so in restricted iframe contexts where safeReplaceState is a no-op, a stale URL value (e.g., tools_q=alpha) would win over the live DOM input (e.g., "beta" that the user just typed).

Inverted the precedence in loadPage():

Primary: DOM inputs via getLiveSearchState(tableName) — always reflects what the user actually typed, works in all contexts
Fallback: URL params — only used when DOM inputs are empty (fresh page load from a bookmarked URL before initializeSearchInputs populates inputs)

Tests updated: the old test "URL params take precedence over DOM fallback" is now correctly "DOM takes precedence over stale URL params (restricted context fix)". Added test for "DOM wins over both stale URL and stale extraParams".

920/920 JS tests pass.

…ial refresh Closes IBM#2992 Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk>

…BM#3647) - Fix TypeError when htmx:afterSettle fires without evt.detail (use optional chaining) - Add 27 tests covering updateFilterStatus, afterSettle rehydration listener, and URL-param filter fallback logic Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

IBM#3647) - loadPage() now trusts DOM input values even when empty (user cleared the filter); URL params are only used as fallback when the DOM element itself is absent (bookmarked URL, iframe, or ID mismatch). Previously an intentional clear during the debounce window fell back to stale URL params and re-sent the old filter. - Simplify updateFilterStatus() to always show "Filters active" instead of attempting to read Alpine-rendered pagination text, which was unreliable due to listener ordering (ran before Alpine.initTree). - Update tests to cover the new element-presence semantics. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

…tle listener (IBM#3647) The htmx:afterSettle listener added by this PR runs at script load time, but admin.js can be loaded in <head> before document.body exists. Use document.addEventListener (consistent with the existing top-level afterSettle listener at line 33807) to avoid the null reference error. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai · 2026-03-21T02:19:48Z

Review complete — rebased, reviewed, tested, approved

Rebase

Rebased onto latest main (aee4056). Resolved one conflict in pagination_controls.html where main's #3128 fix (DOM-based search reading) overlapped with the PR's URL-param approach — merged as DOM-first with URL fallback.

Bugs found and fixed

#	Severity	Issue	Fix
1	Medium	`evt.detail` null access — TypeError when `htmx:afterSettle` fires without `detail`	Added optional chaining: `evt.detail?.target`
2	High	Stale filter on clear — clearing search + paginating before debounce resent old filter (`'' \|\| 'old-value'` = `'old-value'`)	Changed to element-presence check: trust DOM value (even if empty) when element exists; only fall back to URL when element is absent
3	Medium	Fragile pagination text reading — `updateFilterStatus()` tried to read Alpine `x-text` span before Alpine initialized (listener ordering)	Simplified to always show "Filters active" (pagination controls already display the count)
4	High	`document.body` null at script load — top-level `document.body.addEventListener` crashed when script loaded in `<head>`	Changed to `document.addEventListener` (consistent with existing top-level listener at line 33807)

Test coverage

Added 27 differential tests covering all new code:

updateFilterStatus — active/inactive filters, multi-table independence, clearing, missing DOM
htmx:afterSettle rehydration — all 4 target suffixes, unrelated targets, null/missing detail guards
URL-param filter fallback — DOM precedence vs URL fallback, namespace isolation, clear-filter semantics

98/98 vitest tests pass.

E2E verification (Playwright + Docker)

Rebuilt and deployed to localhost:8080 via docker-compose.yml
0 JS console errors in fresh browser session
Search filtering, URL param namespacing, and "Filters active" status all working correctly
No regressions in other admin tabs

Commits

#	Author	Message
1	Oriol Morros Vilaseca	`feat(ui): persist admin table filters across HTMX pagination and partial refresh Closes #2992`
2	Mihai Criveti	`fix(ui): guard afterSettle detail access and add differential tests (#3647)`
3	Mihai Criveti	`fix(ui): prevent stale filter fallback and simplify updateFilterStatus (#3647)`
4	Mihai Criveti	`fix(ui): use document instead of document.body for top-level afterSettle listener (#3647)`

crivetimihai

Approved — rebased, 4 bugs fixed, 27 differential tests added, verified via vitest (98/98) and Playwright e2e (0 JS errors). Ship it.

omorros requested a review from crivetimihai as a code owner March 12, 2026 11:00

marekdano mentioned this pull request Mar 12, 2026

feat(ui): persist admin table filters across HTMX pagination and part… #3204

Closed

10 tasks

marekdano added this to the Release 1.0.0 milestone Mar 12, 2026

marekdano added enhancement New feature or request ui User Interface SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release release-fix Critical bugfix required for the release merge-queue Rebased and ready to merge labels Mar 12, 2026

marekdano assigned crivetimihai Mar 12, 2026

marekdano previously approved these changes Mar 12, 2026

View reviewed changes

crivetimihai dismissed marekdano’s stale review via d949262 March 17, 2026 22:40

crivetimihai force-pushed the feat/persist-admin-table-filters branch from dccbdc0 to d949262 Compare March 17, 2026 22:40

crivetimihai requested review from kevalmahajan and madhav165 as code owners March 17, 2026 22:40

crivetimihai previously approved these changes Mar 17, 2026

View reviewed changes

crivetimihai dismissed their stale review via 812d055 March 17, 2026 23:52

omorros and others added 4 commits March 21, 2026 01:31

feat(ui): persist admin table filters across HTMX pagination and part…

4e9546a

…ial refresh Closes IBM#2992 Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk>

crivetimihai force-pushed the feat/persist-admin-table-filters branch from cd1a5f1 to eb1b732 Compare March 21, 2026 02:19

crivetimihai approved these changes Mar 21, 2026

View reviewed changes

crivetimihai merged commit c3f3191 into IBM:main Mar 21, 2026
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui): persist admin table filters across HTMX pagination and partial refresh#3647

feat(ui): persist admin table filters across HTMX pagination and partial refresh#3647
crivetimihai merged 4 commits intoIBM:mainfrom
omorros:feat/persist-admin-table-filters

omorros commented Mar 12, 2026 •

edited by marekdano

Loading

Uh oh!

marekdano left a comment

Uh oh!

crivetimihai commented Mar 17, 2026

Uh oh!

crivetimihai left a comment

Uh oh!

crivetimihai commented Mar 17, 2026

Uh oh!

crivetimihai commented Mar 18, 2026

Uh oh!

crivetimihai commented Mar 21, 2026

Uh oh!

crivetimihai left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

omorros commented Mar 12, 2026 • edited by marekdano Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related Issue

📝 Summary

🏷️ Type of Change

🧪 Verification

✅ Checklist

📓 Notes (optional)

Uh oh!

marekdano left a comment

Choose a reason for hiding this comment

Uh oh!

crivetimihai commented Mar 17, 2026

Maintainer Review

Conflict Resolution

Test Coverage Added

Review Notes

Uh oh!

crivetimihai left a comment

Choose a reason for hiding this comment

Uh oh!

crivetimihai commented Mar 17, 2026

Additional fix: DOM fallback for restricted iframe contexts

Uh oh!

crivetimihai commented Mar 18, 2026

Fix: DOM-first precedence for filter reads

Uh oh!

crivetimihai commented Mar 21, 2026

Review complete — rebased, reviewed, tested, approved

Rebase

Bugs found and fixed

Test coverage

E2E verification (Playwright + Docker)

Commits

Uh oh!

crivetimihai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

omorros commented Mar 12, 2026 •

edited by marekdano

Loading