OpenTender: identifier-first dispatch + name-equivalence gate (fixes #29) - #42
Closed
Nitjsefnie wants to merge 2 commits into
Closed
OpenTender: identifier-first dispatch + name-equivalence gate (fixes #29)#42Nitjsefnie wants to merge 2 commits into
Nitjsefnie wants to merge 2 commits into
Conversation
Fixes the "query issues" that got OpenTender disabled in b96e681, so the adapter can be re-enabled once the DB artifact ships. - fetch_by_registration(country, registration_number): queries the indexed body_ids table on id_value, scoped to the tender's country and restricted to the id_types that genuinely carry registration numbers (ORGANIZATION_ID / TRADE_REGISTER / HEADER_ICO / TAX_ID). Internal keys (SOURCE_ID / BVD_ID / ETALON_ID) and the near-useless VAT rows are excluded, so a same-digit identifier in another jurisdiction can never produce a false hit. - fetch_by_name(legal_name): name-recall fallback gated on name equivalence exactly like sources/openaleph.fetch_by_name (issue StephenAbbott#21) — a body must actually bear the subject's legal name, so "Orange S.A." never attaches a "Red-Orange e.U." tender. No relevance-score threshold. - Wire both into routers/lookup.py as _opentender_strategies (identifier-first, name-gated fallback), mirroring _openaleph_strategies: OpenTender is a list-result registration+name source, not a single-result _REGISTRY_SOURCES adapter. Keyed on the derived national IDs the pipeline already computes for the 8 built countries (AT/CZ/DK/EE/FI/FR/LT/LV). Dormant until opentender is registered — REGISTRY flip deliberately left out of this commit. - Fix a pre-existing FTS fallback bug in _db_search_impl: names with internal punctuation ("S.A.", "e.U.", "A/S") built the invalid prefix term "S.A.*" and raised fts5: syntax error, silently returning nothing; extract alphanumeric word-runs so the name path works for real company names. Tests build synthetic body_ids/tenders rows through the extractor's own _DDL/_insert_tender machinery. Graceful-absence behaviour (no DB → clean []) is proven for both methods and the pipeline strategy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The strategies-level tests await _opentender_strategies directly and only check _dispatch membership, so a silent drop of "opentender" from the "list-result sources" tuples (the consume-loop in _lookup_pipeline and the /lookup-source retry) would go unnoticed — the hits are computed then discarded. Add two end-to-end tests that drive the real consume paths with a monkeypatch-registered adapter and a fixture DB: - test_opentender_hit_survives_the_pipeline_consume_loop drives _lookup_pipeline (dispatch isolated to opentender; the tuple under test is NOT patched) and asserts the opentender hit event is emitted. - test_opentender_hit_survives_lookup_source_retry drives lookup_source directly and asserts the hit lands in the response. Both fail when "opentender" is removed from their respective tuple (verified), green when present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Closing in favor of #74. Since this was opened, main gained guard tests (test_adapter_is_registered / test_warm_opentender_db_noop_when_not_registered) asserting opentender stays OUT of the registry until the artifact-slimming block in #29 is resolved — this PR's router+registry wiring contradicts them, while #74 lands the identifier-first primitive in the scope #29 currently allows. The wiring half can return as a small follow-up once the artifact issue lands. |
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.
Implements #29 — registration-number dispatch with the name path gated on name equivalence, so
Orangecan never again attach Red-Orange e.U.'s furniture tenders to Orange S.A.What's in it
fetch_by_registration(country, registration_number)— indexedbody_ids.id_valuequery, country-scoped, joined back totenders, restricted to your allowlist (ORGANIZATION_ID,TRADE_REGISTER,HEADER_ICO,TAX_ID— notSOURCE_ID/BVD_ID/ETALON_ID/VAT).openaleph— this is a deliberate deviation from the issue'sLookupDeriverwording, because that machinery is structurally single-dict-result (_collect_registry_sourcesrequires a_bh_<id>()builder and_build_result_hitbuilds exactly one hit from one dict); a multi-hit registration+name source can't be expressed there without rewriting the pipeline's result handling.openalephis your existing precedent for exactly this adapter class, so_opentender_strategiessits beside_openaleph_strategieswith the same registry-gate/dedup shape, identifier-first (derived national IDs for the 8 built countries:at_fn,cz_ico,dk_cvr,ee_registry_code,fi_ytunnus,siren,lt_code,lv_regcode), name as fallback. If you'd rather the country→key map live on the adapter class, that's a trivial move — say so._tender_bears_name, mirroringfetch_by_name's_bears_name/_normalise_namegating from openaleph (c95600f). No relevance thresholds, per OpenAleph: free-text name fallback produces false-positive hits for generic-name entities #21._db_search_impl's fallback tokenized by whitespace-split, so any name with internal punctuation (S.A.,e.U.,A/S,s.r.o.) became an invalid FTS5 query (S.A.*→ syntax error) and silently returned nothing — which would have made the name gate dead for real company names. Now tokenized FTS-safe (\w+). Verified old-vs-new over adversarial name shapes (hyphens, digits, diacritics): everything that worked before still returns the same rows; punctuated names go from silent-empty/error to working. This also improves the existing/search.REGISTRYflip. Your CLAUDE.md registration checklist requires the frontend count copy, README/docs/social updates, and the OKF bundle regen in the same commit (else theokfdrift job goes red) — that's deliberately left as your registration step. What remains: add"opentender": OpenTenderAdapter()toREGISTRY, drop it from_DELIBERATELY_UNREGISTEREDintest_sources.py, and run the checklist. Graceful DB-absence is proven either way (noOPENTENDER_DB_FILE→ adapter methods return[], strategies return[], nothing crashes — tested for both the registered and unregistered states, so flipping is safe even before the artifact is hosted).Tests (17 new; suite 2358 → 2375, 0 failed)
Fixture DBs built through the extractor's own
_DDL/_insert_tender. Your real examples are reproduced as fixtures verbatim — SIREN380129866, bodies namedRed-Orange e.U.andOrange controls s.r.o.rejected while a genuineOrange S.A.passes the gate — plus the id_type exclusion, cross-country non-match, graceful absence, and two end-to-end pipeline-membership pins (an opentender hit must survive the_lookup_pipelineconsume-loop and the/lookup-sourceretry path; each test fails if"opentender"is dropped from its list-result tuple). Both load-bearing behaviors are mutation-verified: deleting the id_type allowlist or bypassing the name gate turns the exact corresponding test red.What this cannot verify (your artifact-side spot-check)
The 5 GB artifact isn't reachable here, so the per-country derived-ID ↔
body_ids.id_valueformat match is confirmed only for FR (siren, from your own worked example). If e.g.at_fn's normalized form differs from how DIGIWHIST stores Austrian registration numbers, the AT path would return nothing on real data. Worth one real subject per non-FR country against the artifact before registering.Generated by Claude Fable 5 (brief, review), Claude Opus 4.8 (implementation)