Add transient retries, facet filters, error logging, and CI - #1
aarontaycheehsien wants to merge 2 commits into
Conversation
Four improvements plus small cleanups: - Retry transient Primo failures (timeouts, connection errors, HTTP 429/5xx) with a short exponential backoff, honouring a numeric Retry-After header capped at PRIMO_REQUEST_RETRY_MAX_DELAY. One extra attempt by default; PRIMO_REQUEST_RETRY_ATTEMPTS=0 disables. - Generic facet_filters/facet_exclusions parameters on primo_search, compiled into Primo's qInclude/qExclude, so callers can act on any facet the "Result landscape" section reports (topic, lang, jtitle, tlevel, ...) instead of only rtype/date/peer_reviewed. Friendly facet name aliases (subject, language, journal, ...) resolve to Primo's names. - Shared error boundary decorator for all MCP tools that logs the traceback on unexpected errors before returning the short message; previously bugs were invisible one-liners. Also hoists the lazy citation/exporter imports. - GitHub Actions CI running the test suite on Python 3.11-3.13. Cleanups: user_agent version now derives from package metadata instead of a hardcoded copy; README documents the retry settings and notes that the recommendation log captures raw query text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wSzXQoB5GD3PSTvF3Ktcp
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01b2752458
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| facet_filters=facet_filters, | ||
| facet_exclusions=facet_exclusions, | ||
| ) | ||
| result = format_search_results( |
There was a problem hiding this comment.
When facet_filters or facet_exclusions are supplied, the search request is correctly filtered, but the formatted Queries run link is built here without those new parameters (and build_search_url has no way to add them). In that case the link in the MCP output points to the unfiltered Primo result set, so users who open or cite it see different counts and records than the tool returned.
Useful? React with 👍 / 👎.
| 'facet_filters/facet_exclusions on a facet value above (e.g. ' | ||
| 'facet_filters={"topic": "..."}), or a narrower query using a ' |
There was a problem hiding this comment.
For rtype and tlevel facets, the values shown above this hint are not always the exact Primo values because _facet_value_label rewrites codes such as online_resources to online and replaces underscores with spaces. Since compile_facet_filters sends the caller-provided value exactly, following this guidance with facet_filters={"tlevel": "online"} or a prettified resource type will silently produce the wrong filter; show the raw value or normalise these display labels before sending them.
Useful? React with 👍 / 👎.
Three recommendation-module improvements: - Semantic matches now carry the profile term whose vector produced the max cosine, so output reads 'Matched by semantic similarity to profile topic "digital preservation" (cosine 0.78)' instead of a bare score, the near-miss evidence names its closest topic, the calibration CLI prints each profile's best term, and recommendation-log entries become triageable without re-running the query. - The parsed sidecar embedding cache is memoised in memory keyed by file mtime (mirroring the directory cache), so semantic calls stop re-reading and re-parsing megabytes of JSON inside the inline search path's latency budget. - Query embeddings get a small LRU keyed by model/prefix/text: paginating results and the zero-result retry policy re-embed identical queries within seconds, and repeats are now free. Injected embedders (tests, experiments) bypass the cache so vectors never leak between backends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wSzXQoB5GD3PSTvF3Ktcp
2 participants
Four robustness/feature improvements to the Primo MCP server, plus small cleanups. Targets
semantic-local-optionsince this branch builds on it.What changed
1. Retry transient Primo failures (
client.py,config.py)Timeouts, connection errors, and HTTP 429/5xx now retry with a short exponential backoff, honouring a numeric
Retry-Afterheader capped atPRIMO_REQUEST_RETRY_MAX_DELAY(5s). One extra attempt by default;PRIMO_REQUEST_RETRY_ATTEMPTS=0disables. Previously the primary search path failed immediately on any blip, unlike the embedding path which already had retry logic. HTTP 400 and other client errors are never retried.2. Generic facet filters on
primo_search(query.py,client.py,server.py,policy.py,formatter.py)New
facet_filtersandfacet_exclusionsparameters ({facet: value}objects) compiled into Primo'sqInclude/qExclude. This closes a gap where the "Result landscape" section surfaced facets (topic, lang, jtitle, tlevel, ...) that callers had no way to act on — onlyresource_type/dates/peer_reviewedwere filterable. Friendly facet-name aliases (subject→topic,language→lang,journal→jtitle, ...) resolve to Primo's names; the landscape hint now points callers at the new params.3. Shared error boundary for all MCP tools (
server.py)A
_tool_error_boundarydecorator replaces the seven copies of try/except. Unexpected (non-PrimoAPIError) exceptions now log a full traceback vialogger.exceptionbefore returning the short caller-facing message — previously genuine bugs were invisible one-liners with no traceback anywhere. Also hoists the lazy citation/exporter imports to module level.4. CI (
.github/workflows/ci.yml)GitHub Actions workflow running the test suite on Python 3.11, 3.12, and 3.13 with uv, on pushes to
mainand all pull requests.Cleanups
user_agentnow derives its version from package metadata (importlib.metadata) instead of a hardcoded0.1.0that could drift frompyproject.toml.PRIMO_RECOMMEND_LOG_FILEcaptures raw user query text on local disk..env.exampleupdated with the retry variables.Testing
429 + Retry-After: 1on first call — confirmed the server retries and returns results, and thatfacet_filters={"subject": "Economics"}/facet_exclusions={"rtype": "reviews"}reach the wire asqInclude=facet_topic,exact,EconomicsandqExclude=facet_rtype,exact,reviews.qInclude=facet_topic,exact,Economics= 60,273;qExclude=facet_rtype,exact,reviews= 710,788; combined include with|,|join = 50,151.🤖 Generated with Claude Code
https://claude.ai/code/session_016wSzXQoB5GD3PSTvF3Ktcp
Generated by Claude Code