fix(search): wrap only boolean queries, probe every portal for the verdict - #534
Merged
Conversation
…rdict Telemetry from the public deployment showed an LLM client reformulating the same request against dati.gov.it for three hours on 29 July. The data was there from the first attempt: `bonifica siti contaminati Piemonte` returns 22 datasets on that portal, the Piedmont contaminated-sites registry first. Our text:(...) rewrite was returning 5. package_search hands a colon-free query to Solr's dismax parser with q.op=AND, mm='2<-1 5<80%' and qf='name^4 title^4 tags^2 groups^2 text' (ckan/lib/search/query.py). dismax has no boolean syntax, so `A OR B` collapses into `A AND B`: on dati.gov.it `aria OR Milano` returns 59, exactly what `aria AND Milano` returns. A colon takes the query off dismax, which is what the wrapper exploits (3421 results). The same switch is why it hurts everything else: it drops the qf boosts that rank titles and tags first, searches the catch-all text field alone, and ANDs every term instead of applying mm. This is CKAN's default, not a per-portal defect. Measured on dati.gov.it: `defibrillatori Comune di Lecce` 678 -> 1, `qualità aria Milano` 650 -> 51, `musei roma arte opere catalogo` 9 -> 0 — the second most repeated query of the semester, answered with an empty page while nine datasets existed. On 40 real queries from telemetry, with a boolean operator the wrapper helped 8 times and hurt none; without one it helped none and hurt 10, six of them down to zero. So the wrapper now applies only to queries carrying a boolean operator. The shape an LLM generates from a user's request — a run of keywords, no operators — goes to the portal's own parser, boosts and mm included. probePortalParser rewritten and now run for every portal, configured ones included; force_text_field removed from portals.json (8 entries), one source of truth. The old probe asked `data OR dati`, common enough to saturate: on Milano `data` alone and `data OR dati` both return 2564, so it read the portal as healthy while `aria OR acqua` returned 0 against 54 and 33 for the single terms. It now picks two terms from the catalog itself and compares A, B, A OR B and text:(A OR B). An OR returning fewer hits than either operand is not being honoured, and the wrapper is the answer only if the wrapped form returns more. data.stadt-zuerich.ch, whose text field returns 0 for every query, is left alone. A plain query pays nothing for this: only a boolean query can be wrapped, so only a boolean query probes. 5 extra rows=0 calls the first time one reaches a portal in a session, then nothing. Verified e2e: dati.gov.it 5 -> 22 and 0 -> 9, Milano `aria OR acqua` 0 -> 87, Zurich unchanged at 10 and 172. 516 tests pass, 18 added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
|
| Filename | Overview |
|---|---|
| src/utils/search.ts | Adds boolean-only wrapping and syntax-aware escaping; the latest escaped-parenthesis fix addresses the previous finding. |
| src/tools/package.ts | Replaces static parser decisions with measured per-portal probing and preserves the resulting override during retries. |
| src/portals.json | Removes stale per-portal search-parser verdicts in favor of runtime detection. |
| tests/unit/search.test.ts | Adds coverage for boolean detection, text wrapping, unary operators, grouping, escaped parentheses, and parser overrides. |
| src/README.md | Documents boolean-only wrapping, runtime portal probing, and its request cost. |
| LOG.md | Records the search regression, parser behavior, implementation strategy, and verification results. |
Reviews (5): Last reviewed commit: "fix(search): a parenthesis the caller es..." | Re-trigger Greptile
… choice Five findings from the review, all confirmed against live portals: - `ambiente -rifiuti` was being wrapped, and escapeSolrQuery turns the exclusion into a requirement: dati.gov.it returns 8047 for `ambiente`, 7649 unwrapped and 398 wrapped, and 8047 - 7649 = 398 — exactly the set the caller excluded. dismax honours +/-/! natively, so a unary operator in operator position now suppresses wrapping entirely, keywords or not. The same characters inside a word still do trigger it: dismax reads `e-government` as `e` NOT `government` (58919 of ~65000 datasets, against 278 for the escaped literal), `COVID-19` 9963 against 69. - the accent-stripping retry resolved the query with params.query_parser, dropping the probe's verdict and re-sending an accented boolean query to the parser that ignores booleans. It now passes parserOverride. - a probe that could not measure — timeout, 500, missing counts — cached `false` for the process lifetime, writing the portal off for the session. Only measured verdicts are cached now. - probe terms are restricted to letters, digits and underscore. A tag like `open-data` carries Solr syntax and would skew the query that measures the parser. - the title fallback now prefers a second term that appears in titles where the first does not. Two words that always travel together make `A OR B` equal `A` even on a healthy portal, caching a false negative. Verified e2e: `ambiente -rifiuti` 7649 with the exclusion intact, `e-government` wrapped to 278, `bonifica siti contaminati Piemonte` 22, Milano `aria OR acqua` 87, accented `qualità aria` 366. 518 tests, 2 added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
Second review round: a mixed query like `aria OR acqua -rifiuti` was skipping the wrapper altogether to protect the exclusion, and silently losing the disjunction on portals whose parser swallows OR. The trade-off was avoidable. Escaping is what broke the exclusion, not the wrapping: `text:(ambiente \-rifiuti)` returns 398 on dati.gov.it — exactly the set the caller excluded — while `text:(ambiente -rifiuti)` returns 7649, the same answer dismax gives. So escapeForTextWrapping leaves a +/-/! in operator position alone and escapes everything else as before. Mixed queries now keep both halves: `text:(aria OR acqua -rifiuti)` returns 1349 against 1389 for the disjunction alone, so the OR is honoured and the exclusion applied. dismax returns 21 for the same disjunction, having swallowed the OR. A query carrying only a unary operator still skips the probe: dismax already honours it, so there is nothing to repair and no reason to pay for the probe. 523 tests, 5 added. Verified e2e: `ambiente -rifiuti` 7649, `aria OR acqua -rifiuti` 1349, `e-government` 278, the Piedmont query 22, Milano 87. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
Third review round pointed at a `-` after `(` being escaped. The premise reaches further: escapeForTextWrapping was escaping the parentheses too, so grouping became literal tokens. That shape is in the telemetry — `(aria OR "qualità dell'aria") AND Milano`, `(Rijkswaterstaat OR RWS) AND (verkeersintensiteit OR INWEVA)` — and on dati.gov.it the first returns 0 through dismax, 51 with the parentheses escaped and 59 with them preserved. So balanced parentheses now survive the escaping and count as term boundaries for a unary operator, which is the reported case. Unbalanced ones are still escaped: stray input must not turn into a Solr syntax error. 526 tests, 3 added. Verified e2e: `(aria OR acqua) AND Milano` wraps to `text:((aria OR acqua) AND Milano)` and returns 98, `aria OR acqua -rifiuti` 1349, `ambiente -rifiuti` 7649 unwrapped, `e-government` 278, the Piedmont query 22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
Fourth review round: `hasBalancedParens` counted escaped parentheses, so `foo OR \(bar\)` was read as balanced grouping. The wrapper then preserved those parentheses while escaping the backslash in front of them, promoting the caller's literal characters to Solr grouping syntax and leaving a stray backslash behind. Both the balance check and the preservation now skip a parenthesis preceded by an odd number of backslashes. Verified e2e, unchanged: `(aria OR acqua) AND Milano` wraps to `text:((aria OR acqua) AND Milano)` and returns 98, `aria OR acqua -rifiuti` 1349, the Piedmont query 22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
aborruso
added a commit
that referenced
this pull request
Sep 5, 2026
Ships #534, the Solr parser fix. Version bumped in package.json, package-lock.json, manifest.json, server.json (both fields), src/server.ts and src/worker.ts. No tools added or removed, so /health stays at 20. 527 tests pass. Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
aborruso
added a commit
that referenced
this pull request
Sep 5, 2026
…e filters Sweeping for defects of the same family as the scoring dilution turned up three more, all local filters running over a truncated or wrongly-normalised set. - ckan_find_relevant_datasets never called the parser probe. portals.json used to cover it, and removing force_text_field left it sending boolean queries to the parser that ignores them: on dati.comune.milano.it `aria OR acqua` returned 0 there against 87 from ckan_package_search. Both tools now share the probe. - ckan_organization_search builds a Solr wildcard, which bypasses the analysis chain, so the pattern has to be pre-normalised the way CKAN builds a name slug. It lowercased but did not fold accents: `città` returned 0 organizations while `citta` matched 135 datasets' worth. - ckan_tag_list applied tag_query after faceting with facet.limit set to the caller's limit. On dati.gov.it 53 tags contain "citta" and none is in the top 100, so the filter answered "no tags" while they existed. openspec/specs/ckan-search/spec.md named ckan_package_search alone in both parser scenarios, which is what let the second caller go unnoticed, and after #534 it was also wrong — it still described the per-portal default that was removed. Rewritten as a property of the query-building path, naming every tool that shares it, with the invariant that a change touching the parser is verified against all of them. `openspec validate --specs --strict` passes. Verified e2e: find_relevant_datasets 87 and 7 on the two boolean queries, matching package_search; `città` finds 4 organizations and 5 tags; the Lecce dataset stays first. 532 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
aborruso
added a commit
that referenced
this pull request
Sep 5, 2026
…ilters (#536) * fix(scoring): score by share of terms, drop Italian stopwords, fix accented boundaries Testing v0.4.122 through an MCP client rather than curl showed ckan_find_relevant_datasets answering its own documented example badly. `defibrillatori Comune di Lecce` on dati.gov.it used to return the catalog's only Lecce dataset — because the search returned exactly one result. With recall restored it returns 679, and the top three became Trento, Martina Franca and Desio while the Lecce dataset fell out of the window. The wrapping fix did not cause this, it removed the cover. Three older defects: - scoreTextField awarded the whole field weight when any term matched, so "Comune di Martina Franca" and "Comune di Lecce" both scored a full holder match and the right dataset could not outrank the wrong ones. It now scores the share of terms the field carries. - the stopword list was English-only, so `di` counted as a term and "Provincia Autonoma di Trento" earned a full holder match on a query asking for Lecce. - `\b` is ASCII-only in JavaScript, so a term ending in an accented letter never found its word boundary: `mobilità` scored 0 against "mobilità urbana", `qualità` against "qualità dell'aria". On a mostly non-English catalog this sank every accented query. Replaced with Unicode lookarounds. The candidate window is now at least 50: the local ranking only sees what Solr returns first, and `limit: 3` shrank it to 15. Verified: the Lecce dataset is first again, `qualità dell'aria Milano` returns air quality monitoring datasets. 532 tests, 5 added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * fix(search): same parser probe for find_relevant_datasets, accent-safe filters Sweeping for defects of the same family as the scoring dilution turned up three more, all local filters running over a truncated or wrongly-normalised set. - ckan_find_relevant_datasets never called the parser probe. portals.json used to cover it, and removing force_text_field left it sending boolean queries to the parser that ignores them: on dati.comune.milano.it `aria OR acqua` returned 0 there against 87 from ckan_package_search. Both tools now share the probe. - ckan_organization_search builds a Solr wildcard, which bypasses the analysis chain, so the pattern has to be pre-normalised the way CKAN builds a name slug. It lowercased but did not fold accents: `città` returned 0 organizations while `citta` matched 135 datasets' worth. - ckan_tag_list applied tag_query after faceting with facet.limit set to the caller's limit. On dati.gov.it 53 tags contain "citta" and none is in the top 100, so the filter answered "no tags" while they existed. openspec/specs/ckan-search/spec.md named ckan_package_search alone in both parser scenarios, which is what let the second caller go unnoticed, and after #534 it was also wrong — it still described the per-portal default that was removed. Rewritten as a property of the query-building path, naming every tool that shares it, with the invariant that a change touching the parser is verified against all of them. `openspec validate --specs --strict` passes. Verified e2e: find_relevant_datasets 87 and 7 on the two boolean queries, matching package_search; `città` finds 4 organizations and 5 tags; the Lecce dataset stays first. 532 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * fix(search): acronyms survive stopwords, NFC matching, unbounded tag facet Three review findings, all confirmed: - the Italian stopwords were applied to every portal, so `UN population` lost the `UN`. Removing the list is not an option — without it the Lecce query ranks Martina Franca first, since `di` inflates its title match — so an all-caps token now survives the list: an acronym is not an article. - term matching compared raw Unicode, so an NFC query would miss NFD metadata. Both sides are normalised to NFC now. - widening the tag facet to 1000 moved the false-negative boundary instead of removing it. `facet.limit: -1` returns every tag when a filter is given: 14138 on dati.gov.it, against 3 from the tag_list action. 535 tests, 3 added. Verified e2e: the Lecce dataset stays first with `di` still dropped, `UN population` keeps its acronym, `città` matches across the full tag set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * perf(tags): bound the tag facet, widen it only when the filter comes up short `facet.limit: -1` made every filtered tag query download the whole facet — 1.4 MB and 0.5s on dati.gov.it, more on a larger catalog — however few tags the caller asked for. The filtering cannot move server-side: CKAN's parameter whitelist rejects Solr's `facet.contains` with "Invalid search parameters". So the facet starts bounded at max(limit * 20, 500), roughly 50 KB, and only a filter that came up short pays for the full set. `citta` is answered by the bounded window, `zzzqwerty` escalates and still returns nothing, which is the honest answer. The escalation exposed one more thing: Solr sorts a facet by count only while facet.limit is positive. Asked for -1 it returns index order, so the widened set arrived reverse-alphabetical ("zuglio", "zucs", ...) and the first three matches were arbitrary. `facet.sort` is rejected by CKAN as well, so the ordering the tool documents is restored locally: `acqua` now returns acqua(154), impianti-agricoli-e-di-acquacoltura(66), acqua-dolce(13). 535 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * fix(tags): let an escalation failure surface, order imports - the exhaustive facet request swallowed its error with `.catch(() => null)` and returned the bounded set, answering "these are the matching tags" while hiding the ones it never looked at — the same silent truncation this PR is about. The error now goes to the tool's normal path. - the stripAccents import sat after the type-only one, splitting the internal group. AGENTS.md orders imports external, internal, types. 535 tests. Verified: `acqua` still returns acqua(154), impianti-agricoli-e-di-acquacoltura(66), acqua-dolce(13); a filter matching nothing still returns nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 6, 2026
aborruso
added a commit
that referenced
this pull request
Sep 6, 2026
…dence DECISIONS.md still described the parser probe replaced in #534 — `data OR dati`, a 2× threshold, configured portals skipping it — and attributed the wrapping to a Solr `df` bug. The measured cause is CKAN's dismax with q.op=AND, and the probe now runs for every portal on terms taken from the catalog. Rewritten with the numbers from #534. Also records three decisions taken this week that had a rule in CLAUDE.md but no "why" anywhere: migrated portals stay listed so the notice keeps firing (#540); the release gate asserts which dataset comes back, not how many (v0.4.122); and a release does not go out the same day as the change that motivates it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What the telemetry showed
On 29 July an LLM client spent three hours reformulating the same request against
www.dati.gov.it/opendata:The data was there from the first attempt. That query returns 22 datasets on the portal, the Piedmont contaminated-sites registry (ASCO) first. We were returning 5.
Why
package_searchhands a colon-free query to Solr's dismax parser withq.op=AND,mm='2<-1 5<80%'andqf='name^4 title^4 tags^2 groups^2 text'(ckan/lib/search/query.py). dismax has no boolean syntax, soA OR Bcollapses intoA AND B— on dati.gov.itaria OR Milanoreturns 59, exactly whataria AND Milanoreturns. A colon takes the query off dismax, which is what wrapping it astext:(...)exploits.The same switch is why the wrapper hurts everything else: it drops the
qfboosts that rank titles and tags first, searches the catch-alltextfield alone, and ANDs every term instead of applyingmm.This is CKAN's own default, not a defect of particular portals — which is why the same pattern shows up everywhere:
On dati.gov.it:
defibrillatori Comune di Lecce678 → 1,musei roma arte opere catalogo9 → 0 — the second most repeated query of the semester, answered with an empty page while nine datasets existed.On 40 real queries from telemetry the separation is clean:
OR/AND/NOTSix of the ten go to zero results.
Changes
The rule (
src/utils/search.ts) — the wrapper is reserved for the queries dismax cannot serve:bonifica siti contaminati Piemonteqfwithmm; wrapping drops botharia OR Milanoq.op=ANDswallows the ORambiente -rifiutie-government,COVID-19title:(aria OR acqua)An explicit
query_parser: "text"from the caller stays a literal override.The escaping (
escapeForTextWrapping) — wrapping only helps if it preserves what the caller wrote.escapeSolrQueryescapes every special character, which inverts a unary operator:text:(ambiente \-rifiuti)returns 398 on dati.gov.it, exactly the set the caller excluded, against 7649 for the unescaped form. So a+/-/!in operator position and balanced grouping parentheses survive; everything else is escaped as before, including a parenthesis the caller escaped, which is a literal rather than a group.This is what lets a mixed query keep both halves:
text:(aria OR acqua -rifiuti)returns 1349 against 1389 for the disjunction alone, so the OR is honoured and the exclusion applied. dismax returns 21, having swallowed the OR. And on a real grouped query from the telemetry,(aria OR "qualità dell'aria") AND Milano: dismax 0, escaped parentheses 51, preserved 59.The probe (
src/tools/package.ts) — rewritten, and now run for every portal, configured ones included.force_text_fieldis gone fromportals.json(8 entries) so there is one source of truth; the stored values had gone stale, with two portals marked "no wrapping" for a reason the rule now handles.The old probe asked
data OR dati, two words common enough to saturate: on Milanodataalone anddata OR datiboth return 2564, so it read the portal as healthy whilearia OR acquareturned 0 against 54 and 33 for the single terms. It now picks two terms from the catalog itself — single-word tag facets between 0.5% and 30%, falling back to frequent title words, restricted to letters, digits and underscore so no Solr syntax leaks into the probe — and comparesA,B,A OR B,text:(A OR B). An OR returning fewer hits than either operand is not being honoured, and the wrapper is the answer only if the wrapped form returns more.data.stadt-zuerich.ch, whosetextfield returns 0 for every query, is correctly left alone.A verdict is cached only when it was actually measured, so a portal that times out or 500s is retried rather than written off for the session. The title fallback prefers a second term appearing where the first does not, since two words that always travel together make
A OR BequalAeven on a healthy portal.Cost — a plain query pays nothing, since nothing but a boolean query can be wrapped. Measured from the audit log: 1 call for a plain search, 6 for the first boolean search on a portal (5 probe + 1 real), 1 afterwards.
Verification
bonifica siti contaminati Piemontemusei roma arte opere catalogoaria OR acqua(Milano)ambiente -rifiutiaria OR acqua -rifiuti(aria OR acqua) AND Milanoe-governmentOut of scope, found on the way
catalog.data.govanswers 404 onpackage_search, anddati.arpae.itis intermittent — 200 on one call, 500 on the next, wrapped or not. Both are configured portals and deserve their own issue.data.gov.ukis fine: it answers through its configured/api/actionpath.🤖 Generated with Claude Code
https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2