Catalog: stop the facet "Sort by" control vanishing as you type (5217 stack 4/9)

[nl0]

Description

The search sidebar's "Sort by" control disappeared while you typed in "Find
metadata". On a stack whose facet list the server truncates, narrowing the list
dropped the count the control gates on below its threshold, and the control
unmounted mid-word — while the reader was using it to hunt for a field. It was
also offered above an empty list, where it has nothing to sort.

This is the visibility half of the "Sort by" work. The control's accessible name
is PR 3, a different file and a
different concern.

Review findings addressed

This unit drew most of the review's attention, and the approach here is the one
f5
asked for: fix the count, rather than patch the symptom twice.

• f5 — visibility was patched in two places at once: a Math.max guess at
  the total and a monotonic per-mount ref that latched the highest count it had
  ever seen. Only one of the four call sites was actually passing a total that
  moved. Fixing that one caller made both patches unnecessary.
• f36 (thread)
  — the latch wrote maxSeen.current in the render body, which React's
  "Referencing values with refs" documents as something not to do, and it was not
  scoped to the narrowing it was written for: any drop in the total made it
  stick for the life of the mount. The ref is gone; useOrderingOffered is now
  orderingOffered, a pure predicate.
• f6 — the count change was itself an untested behaviour change. Five tests
  now drive the predicate directly, including the narrowing case that used to
  flicker and the withhold-on-empty case.
• f47 — the gate's contract was documented in three places (the state
  declaration, the totalAvailable prop, the consumer in PackageFilters.tsx)
  and all three had gone stale against the implementation. They describe it
  correctly again, and the withhold-on-empty rule — which was never written down
  anywhere — is now stated.
• f3 — the old description claimed the code "passes the pre-filter count"
  while shipping Math.max(initial.length, available.length), whose own comment
  conceded neither operand is the pre-filter total. It now passes initial.length,
  and the claim is true as stated.
• f4 — and to state it plainly rather than let it be inferred: this changes
  the visibility rule on all four paths through
  AvailablePackagesMetaFiltersGroup, not the one path the old description named.
  Three of them already passed a stable pre-filter count and are unaffected by the
  count change, but all four now go through the same predicate and all four gain
  the withhold-on-empty rule.
• f7 — the totalAvailable prop doc said "the count before any filtering",
  which was offered as the substitute for a test and which the diff falsified. It
  now says what the value is: a lower bound that excludes already-applied facet
  filters and understates a truncated list, whose one load-bearing property is
  that it does not move while the filter box is typed in.

On f5, and what was not available

f5 recommended gating on "the count the server already answers". It does not
answer one. PackagesSearchStats exposes userMeta (the possibly-truncated
list) and userMetaTruncated (a boolean) — there is no facet total in the schema;
PackagesSearchResultSet.total counts packages, not fields. So the fallback f5
allowed for applies, and initial.length is the strongest sound stand-in the
client can compute: stable across the search, and a genuine lower bound because
the list reaching that path is truncated by definition.

Verification

cd catalog && npx vitest run app/containers/Search

107 tests. The five in model.spec.ts under orderingOffered are the ones this
PR adds.

Position in the stack

PR 4 of 9, based on
stack/5217-3-search-ordering-accessible-name.

Part of the split of #5217 asked for in
f27. It
stacks directly above PR 3 because f47's fix corrects a comment in
PackageFilters.tsx, which PR 3 owns — stacking avoids a cross-branch edit. The
two are deliberately not collapsed: PR 3 is an uncontested accessibility fix and
should not need this unit's review to land.

TODO

• Unit tests
• Security: Confirm that this change meets security best practices and does not violate the security model
• Open: Confirm that this change doesn't break the Open variant
• Changelog entry

🤖 Generated with Claude Code

Greptile Summary

This PR stabilizes the metadata-facet “Sort by” control while users narrow truncated facet lists and hides it when no facets are displayed. It also unintentionally removes contextual malformed-filter error handling and associated regression tests.

• Uses the stable initial facet-list length for the server-filter ordering threshold.
• Centralizes ordering-control visibility in the pure orderingOffered predicate.
• Adds focused predicate coverage and updates explanatory comments and the changelog.
• Replaces contextual filter parsing with direct JSON.parse calls, degrading malformed-URL errors.

Confidence Score: 4/5

The malformed-filter error regression should be fixed before merging so invalid shared or bookmarked search URLs retain actionable user-facing errors and diagnostics.

Direct JSON.parse calls now send raw syntax messages through the search error boundary and omit the malformed value from logs, replacing the existing filter-specific error contract.

Files Needing Attention: catalog/app/containers/Search/model.ts, catalog/app/containers/Search/model.spec.ts

Important Files Changed

Filename	Overview
catalog/app/containers/Search/model.ts	Correctly centralizes ordering visibility, but regresses malformed search-URL handling by removing contextual parsing and logging.
catalog/app/containers/Search/model.spec.ts	Adds useful ordering visibility cases but deletes malformed-filter regression coverage alongside the behavioral regression.
catalog/app/containers/Search/Layout/PackageFilters.tsx	Updates the rendering comment to accurately describe centralized ordering visibility.
catalog/CHANGELOG.md	Documents the intended facet-ordering visibility fix.

_{Reviews (1): Last reviewed commit: "docs(changelog): entry for the facet ord..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Catalog client features

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 35.16%. Comparing base (0dcf150) to head (ec3d8f9).

Files with missing lines	Patch %	Lines
catalog/app/containers/Search/model.ts	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                              Coverage Diff                              @@
##           stack/5217-3-search-ordering-accessible-name    #5262   +/-   ##
=============================================================================
  Coverage                                         35.16%   35.16%           
=============================================================================
  Files                                               741      741           
  Lines                                             23806    23808    +2     
  Branches                                           6429     6431    +2     
=============================================================================
+ Hits                                               8371     8373    +2     
+ Misses                                            13692    13691    -1     
- Partials                                           1743     1744    +1     

Flag	Coverage Δ
catalog	35.16% <66.66%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[greptile-apps]

Contextual filter errors removed

When a shared or bookmarked search URL contains malformed filter JSON, the direct JSON.parse call now sends a raw syntax error through the search error boundary, causing the page to display a context-free parsing message instead of identifying the invalid filter; removing parseFilterJson also drops the malformed value from diagnostic logging. The same regression applies to the number-range and keyword-list parsers.

Knowledge Base Used: Catalog client features

[nl0]

Accepted — fixed in 39c2aac.

Correct, and it was wider than this call site. The layer also dropped parseFilterJson itself, the Log import in both model.ts and model.spec.ts, and the six-test describe('Predicates: malformed filter JSON') block — a straight revert of #5233, which is an ancestor of this PR's base. None of the four commit messages mentions any of it, so these two files look reconstructed from a pre-#5233 state rather than patched.

Restored all of it: the helper, the three call sites in Predicates.Datetime, Predicates.Number and Predicates.KeywordEnum, both Log imports, and the tests. This layer's own orderingOffered work is untouched — the two regions sit at opposite ends of the file. model.spec.ts is back to 45 tests, the Search suite is 114 green (was 108), and this layer's diff against its base is now ordering-only.

Worth adding the changelog half: the #5233 entry was still present in catalog/CHANGELOG.md, so as it stood the stack would have shipped the claim without the behaviour.

[nl0]

Local code review — findings and dispositions

Reviewed this layer only (stack/5217-3-search-ordering-accessible-name..stack/5217-4-search-ordering-visibility, 4 commits, 4 files), alongside the automated review on this PR.

Accepted

1. This layer reverted #5233's malformed-filter-JSON handling. (also flagged inline by the automated review)

parseFilterJson, its three call sites in Predicates.Datetime / Predicates.Number / Predicates.KeywordEnum, the Log import in both model.ts and model.spec.ts, and the six-test describe('Predicates: malformed filter JSON') block were all removed, with no replacement in the layer and no mention in any of the four commit messages.

It is user-visible. SearchErrorFallback and PackageListErrorFallback render error.message verbatim, so a shared or bookmarked /search?modified={ showed Unexpected end of JSON input rather than Invalid date range in the search URL, with nothing naming the parameter at fault. The Log.error line carrying the offending value was gone too, so all three filter types collapsed into indistinguishable Sentry groups with no payload. Meanwhile the #5233 entry was still in catalog/CHANGELOG.md, so the stack would have shipped the claim without the behaviour. The deleted tests are exactly the ones that would have caught this, which is why the suite stayed green.

Fixed in 39c2aac, restored from this PR's base. The ordering work is untouched — the two regions sit at opposite ends of the file.

2. Comments narrated the change rather than the code.

The orderingOffered docblock and two of its tests described an earlier useOrderingOffered that latched the highest count in a ref, and the flicker the predicate replaces. That version never shipped, so a reader of this file cannot reach it and has no reason to. Trimmed in 07a0f23 to what the code cannot show: why the threshold and the withhold rule read different counts, why the truncated path passes initial, and that the predicate must answer the same whoever asks.

Declined

The threshold no longer governs the count actually on screen. On the truncated path the control now stays mounted above a one- or two-field list, so FACET_ORDERING_THRESHOLD gates the pre-filter total rather than what is displayed. That is the PR's stated intent — it is what stops the flicker — and it matches the client-filter path's existing behaviour, so the stack is internally consistent. Not a defect.

Reviewed and found clean

• totalAvailable: initial.length — initial derives from the outer META_FACETS_QUERY, not the per-keystroke META_FACETS_FIND_QUERY, so it genuinely does not move while the filter box is typed in; the path is reached only when the list is truncated, so it is a real lower bound rather than a guess.
• orderingOffered(totalAvailable, available?.length ?? 0) — available is non-null on every Ready path that reaches the group, and the ?? 0 correctly withholds on non-Ready states, which do not render the control anyway.
• Replacing the render-time ref latch with a pure function is correct; the other call sites already passed pre-filter counts.
• Changelog entry number and ordering are consistent with the layers below.

Verification

npx tsc --noEmit clean · npx oxlint app clean · npx vitest run app/containers/Search 11 files / 114 tests green (was 108) · full suite at the stack tip 181 files / 1722 passed, 1 skipped.

Layers 5–9 were rebased onto the updated branch in order; git range-diff shows every commit in each of them content-identical across the rebase, and each PR still shows only its own layer.