You're raising valid points. Looking at the code:
On (a) - why only one instance:
There are actually two parallel filter systems here:
normalizeFilter → single filter value (used for the toggle between "Highlights"/"Everything" mode)parseFiltersFromUrl → multi-value filters[] + toolTypes[] (used for the sidebar checkboxes)
parseFiltersFromUrl does its own validation internally (line 54-57 filters against VALID_FILTERS). So normalizeFilter only exists for the single-value case.
On (b) - why not inside buildFilterUrl:
buildFilterUrl is for writing to URLs, not reading. The values passed to it are already typed as MessageFilter[] from code — they're trusted. Sanitization is only needed when reading from URLs (untrusted user input).
That said — you're right this looks messy.
Looking at lines 68-73, there's redundancy: both normalizeFilter(rawFilter) and parseFiltersFromUrl(searchParams) are called, and they're both reading ?filter= params. The single filter variable and the parsedFilters array are used for different things but overlap conceptually.
This looks like transitional code — probably from migrating the old single-filter toggle to a multi-filter sidebar. If the multi-filter approach is the intended future, normalizeFilter could likely be deleted and replaced with deriving a "mode" from parsedFilters (e.g., parsedFilters.includes("highlights")).
Want me to check the project doc to see if this is intentional or cleanup fodder?
