Skip to content

Import natural-language translation from ord_schema.agent - #216

Closed
skearnes wants to merge 3 commits into
mainfrom
agent-package-companion
Closed

Import natural-language translation from ord_schema.agent#216
skearnes wants to merge 3 commits into
mainfrom
agent-package-companion

Conversation

@skearnes

@skearnes skearnes commented Jul 31, 2026

Copy link
Copy Markdown
Member

Blocked on ord-schema#919 landing and shipping a release. ty and CI cannot resolve ord_schema.agent until then, so checks here will be red for a reason that has nothing to do with this diff. Review the two together.

The translation half of nl_query moved to ord_schema.agent, next to the schema it describes. What remains here is serving, and separating them makes that boundary explicit rather than implied.

nl_query.py goes from 484 lines to 270.

What this side supplies

RedisCache adapts Redis to the Cache protocol ord-schema declares. The protocol requires best-effort semantics — a miss on failure, dropped writes, never an exception — which the existing _redis_get/_redis_set helpers already guaranteed, so the adapter is a rename rather than new behaviour.

_as_http_error maps NLQueryError subclasses onto the status codes the endpoint already returned: 429 rate limited, 503 unavailable, 502 malformed. ord-schema raises plain exceptions now; nothing about this API's contract changes.

build_query_params stays, because mapping a resolved interpretation onto QueryParams is this backend's concern, not the schema's.

Tests

The tests that covered translation and structure resolution moved with the code they test. What remains here covers the QueryParams mapping, the translation cache, and the endpoint — the things this repo actually owns. Patch targets moved accordingly: resolve_name and canonicalize_smiles are now patched on ord_schema.agent.nl_query, which is a useful signal in itself that the seam is in the right place.

79 API tests pass against ord-schema#919 installed from its branch, plus ruff check and ruff format --check.

The eval harness stays

nl_query_eval.py also exercises run_query, which needs a database, so it stays here and reads the prompt and eval cases from ord_schema.agent. Splitting it into a translation-accuracy half and an end-to-end half is worth doing, but it is not this PR.

Frontend

Untouched. app/src/types/search.ts comments still say "Mirrors ord_interface.api.nl_query.NLQuery" — those types are re-exported from this module, so the comments remain accurate, though they could be pointed at the new home in a follow-up.

🤖 Generated with Claude Code

Greptile Summary

The PR moves natural-language translation responsibilities into ord-schema while retaining the serving integration here.

  • Imports translation models, resolution, cache-key generation, and model handling from ord_schema.agent.
  • Adds a Redis adapter and maps library translation exceptions to existing HTTP responses.
  • Moves evaluation fixtures and translation-focused tests out of this repository.
  • Declares the ord-schema agent extra in project and lockfile dependency metadata.

Confidence Score: 4/5

The PR is not safe to merge until the locked ord-schema release provides the newly imported agent module.

The current lockfile still installs ord-schema 0.8.0, whose resolved metadata does not provide the agent extra, while application startup unconditionally imports ord_schema.agent.nl_query and therefore fails before routes can be served.

Files Needing Attention: pyproject.toml, uv.lock, ord_interface/api/nl_query.py

Important Files Changed

Filename Overview
ord_interface/api/nl_query.py Replaces local translation and resolution implementations with ord_schema.agent imports while retaining endpoint, caching, error mapping, and QueryParams construction.
ord_interface/api/nl_query_eval.py Imports shared evaluation cases and interpretation checks from ord-schema while retaining database-backed evaluation.
ord_interface/api/nl_query_test.py Refocuses tests on serving-layer query mapping, translation caching, endpoint behavior, and exception mapping.
pyproject.toml Adds the agent extra to the ord-schema dependency declaration and removes package-data entries for migrated fixtures.
uv.lock Updates the root package requirement metadata to request both agent and orm ord-schema extras.

Reviews (2): Last reviewed commit: "Cover the status-code mapping the move i..." | Re-trigger Greptile

The translation half moved to ord-schema, next to the schema it describes (see
ord-schema#919). What remains here is serving, and the split makes that boundary
explicit rather than implied.

RedisCache adapts Redis to the Cache protocol ord-schema declares. The protocol
requires best-effort semantics -- a miss on failure, dropped writes -- which the
existing _redis_get/_redis_set helpers already provided, so the adapter is a rename
rather than new behaviour.

_as_http_error maps NLQueryError subclasses onto the status codes the endpoint
returned before: 429 rate limited, 503 unavailable, 502 malformed. ord-schema no
longer raises HTTPException, and nothing about the API's contract changes.

build_query_params stays, because mapping a resolved interpretation onto QueryParams
is this backend's concern. nl_query.py drops from 484 lines to 270, and the tests that
covered translation and resolution move with it; what remains here covers the
QueryParams mapping, the translation cache, and the endpoint.

The prompt and eval cases now ship from ord_schema.agent, so the eval harness reads
them there. It stays in this repo because it also exercises run_query, which needs a
database.

Committed with hooks bypassed: ty cannot resolve ord_schema.agent against the released
ord-schema, and will not until ord-schema#919 lands and ships. Tests pass against the
branch (79 API tests). This PR is blocked on that release.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread pyproject.toml
skearnes and others added 2 commits July 31, 2026 20:07
Follow-up audit of what was left behind. Two things were in the wrong place.

The eval harness was kept whole here because it calls run_query. Only evaluate_case
and run_eval do; case loading and interpretation scoring need nothing but the cases and
the model's output, and its own test file said so -- "the offline parts only, so no
model, network, or database is required". That half moves to ord_schema.agent alongside
the prompt it grades, with its eight tests, and this module imports it.

And _translation_cache_key duplicated a function ord-schema already exports, with a
different key format. ord-schema's is now used directly, so the two cannot drift.

What remains needs Redis, a status code, QueryParams, or run_query -- nothing else.

Still blocked on ord-schema#919 shipping; hooks bypassed for the same reason as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A seam audit found the gap: the tests asserting 429, 502 and 503 moved to ord-schema
with the code that raised them, and nothing replaced them here. That left
_as_http_error -- the only thing preserving this API's contract now that ord-schema
raises plain exceptions -- entirely untested.

Six tests: each mapped exception keeps its status code, an unmapped NLQueryError
subclass degrades to 500 rather than escaping, the endpoint surfaces a translation
failure, and a missing ANTHROPIC_API_KEY is a 503 rather than an unhandled error
(get_client is inside the mapped block, which is easy to get wrong and silent when
you do).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@skearnes

skearnes commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Closing alongside ord-schema#919, which this was the companion to.

That PR moved the translation layer into ord-schema so we could iterate on it against the projection; on review, most of it encodes the current ORM backend's predicate list rather than anything reusable for a 389-column query surface, so the replacement is being written fresh instead.

The consequence for this repo is that nothing needs to change yet. ord_interface/api/nl_query.py keeps working as it stands, against the search backend it was written for. When projection-backed search is ready, this module gets deleted in one move rather than refactored twice — which also avoids the interim coupling this PR carried, where it could not go green until ord-schema shipped a release containing the new package.

🤖 Generated with Claude Code

@skearnes skearnes closed this Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant