[WIP] Canonical v2 migration#80
Open
sergeyf wants to merge 81 commits into
Open
Conversation
- work_plan §2: 2026-07-04 reachability audit note; refresh drifted line references; move the same-signature paper_author_list item to Tier B (the residual-summary training path already excludes the query) - normalization_migration_blocked: unblocking status, Open Decisions 3-4 (benchmark training-data names; single-mode cutover), implementation notes from the review, gate re-baselining protocol, threshold scope - production_inference: new bundles must store runtime eps in clusterer.json; do not extend the v1.2 override list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tests/fixtures/canonical_name_examples.json: 41 cases across hyphen/space given names, apostrophe variants, initials, compound and particle surnames, title prefixes, transliteration, and missing-field edges; each case carries the proposed canonical_v2 fields + count keys, the computed legacy values, and references into a decisions registry (D1-D8, decided vs open) - tests/test_canonical_name_examples.py: pins the legacy values against the current normalizer and count-key shims (runs now, guards against legacy drift on this branch), checks table coherence (equivalence groups, decision references, normalized-form invariants), and asserts the canonical contract against canonicalize_name_parts / canonical_name_count_keys (skipped until migration step 2 lands) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- D1 (ruled): spill on space, keep together on dash - the current dash-aware splitter is confirmed as canonical policy; cross-variant compatibility stays compare-time via same_prefix_tokens - D3 (ruled): all apostrophe-like marks including backtick are removed from name fields - D6 (refined): count keys are the canonical fields verbatim, the single-char informative gate is unchanged, missing components yield null keys instead of sentinel counts - Fixture regenerated: field-level canonical deltas now reduce to the apostrophe cases only; count-key deltas to decompaction + nulls - Migration doc: rulings recorded in the Decided list; step 1 marked complete with a pointer to the fixture Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Legacy count keys now asserted through the LIVE ANDData._compute_signature_name_counts path with seeded count dicts (drift in the real method fails the test), replacing the mirrored key-construction helper - Add tested compare-time contract: canonical firsts across Jo/Jo Ann/ JoAnn-style variant groups must be pairwise same_prefix_tokens- compatible, with negative pairs guarding against degenerate compatibility - D6 reframed explicitly as a feature-semantics change (null keys for missing components change live lookups) gated by the retrain re-baseline eval; D7 prefix-only edge worded as cleanly decided; migration doc Decided list aligned Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Prescribed, decision-free fixes from the deferred correctness batch, with the one genuine policy call (detect_language single-detector reliability) left open. Parity pairs (both implementations): - self-cite: guard on paper_id_1 != paper_id_2 (featurizer.py, rust_featurizer.rs) - email: absent '@' -> None suffix / NaN feature (text.email_prefix_suffix, features.email_parts) - equal_middle: single initial vs the set of all token initials (text.py, features.middle_names_equal) - whitespace-only equal: strip-then-test-empty (text.equal, features.first_names_equal) - compute_ref: reference-list features computed without reference_details; only the 4 ngram counters still gated (featurizer.py, rust_featurizer.rs) Python-only: - ORCID pattern uses [0-9] (reject Unicode digits), matching Rust - empty-surname name counts return np.nan for last-dependent keys (D6) - get_text_ngrams short-token filter decoupled from stopwords - apply_sinonym_overwrites invalidates derived normalized/name-count fields - same-signature guard moved above paper_author_list_* updates (Tier B) Rust-only: - raw_arrow readers route required string columns through required_value - extract_specter_vec keeps all-zero rows as present (matches Arrow ingest) FEATURIZER_VERSION 3->4 (invalidates the pair-feature cache). New coverage in tests/test_text.py and tests/test_correctness_pass.py; canonical fixture's legacy count keys now use None for no-lookup components via the live ANDData path. work_plan section 2 records the pass and the remaining open item. Verified: 343 python tests + 23 parity/correctness tests + 28 rust unit tests pass; ruff clean; maturin build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build_name_counts_data_from_artifact defaulted last / first_last / last_first_initial to the sentinel 1.0 on a lookup miss, with no guard for an empty surname. After the 2026-07-04 Python fix made ANDData._compute_signature_name_counts return np.nan for every last-dependent key when the surname is empty (D6), the two implementations diverged: for an empty-surname signature routed through the Rust staged-preprocessing artifact path, Python reported NaN while Rust reported a corpus count of 1. The existing parity tests missed it because none exercise an empty surname through this path. Add the same first_informative / last_present gates the Python method uses, so an empty surname yields NaN in both. New self-contained Rust regression test (empty_surname_yields_nan_matching_python) builds a zero-record on-disk index so has_data() is true and drives both an empty- and present-surname case. Verified: 29 rust unit tests pass; 154 python parity/name-count/correctness tests pass after maturin rebuild. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 2026-07-04 pass routed the required Arrow string columns (author first/middle/last/suffix, title/venue/journal_name) through required_value, which ERRORS on NULL. That was a false premise: the Arrow producer's _optional_str maps both "" and None to Arrow NULL, so NULL is the normal representation of an empty value -- author_suffix is ~100% NULL in real bundles and empty surnames are valid per D6. required_value would raise on essentially the first row of every real dataset; parity fixtures missed it because they hand-write "" instead of NULL. Revert those 7 columns to optional_owned().unwrap_or_default() (NULL -> ""), restoring Python<->Rust parity (Python coerces None -> "" via normalize_text). Add a self-contained Rust round-trip test (null_paper_strings_read_as_empty_not_error) building a papers batch with NULL title/venue/journal_name and asserting they read back as "". abstract stays optional. No FEATURIZER_VERSION change (feature values unchanged). Verified: 30 rust unit tests; 97 python parity/arrow-ingest tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 2026-07-04 correctness pass moved the `if same_signature: continue` guard above the paper-author-list updates (the 'Tier B hardening' item), so a query that is a member of its own candidate cluster stopped contributing to the paper_author_list_* features. That broke a pre-existing, passing test, tests/test_query_adapter.py::test_local10_evidence_ignores_query_signature_member, which deliberately locks the intended behavior: paper-author-list features DO include the self-match (jaccard 1.0); only the local10 window excludes the query signature. The correctness-pass verification never ran test_query_adapter.py, so the regression slipped through. The guard was speculative hardening for a caller that does not exist (production incremental linking and linker training both drop the query upstream, so no live path feeds a query-inclusive summary) -- which CLAUDE.md disallows. Restore the original placement: skip same_signature only for the local10-window features. The work_plan section-2 note and the Tier B entry are corrected in the accompanying docs update. Verified: test_query_adapter.py 13 passed; full suite 1137 passed, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Part of the v1.3 canonical-name migration: canonical upstream normalization replaces the Sinonym per-paper detect-and-overwrite step, so the Sinonym code path, its dependency, and its tests are removed. - s2and/data.py: drop _SINONYM_DETECTOR/_SINONYM_DETECTOR_LOCK, the use_sinonym_overwrite ANDData option, and apply_sinonym_overwrites (~440 lines net). - s2and/text.py: reword a Sinonym comment to 'normalized'. - pyproject.toml / uv.lock: drop the sinonym dependency. - scripts (_rust_suite/*, analyze_giant_block_subblocking, convert_to_arrow, linker_train_calibrate_eval, tutorial, verification/*) and incremental_linking_training/data_loading.py: drop the use_sinonym_overwrite argument. - tests: delete test_sinonym_overwrite.py and test_sinonym_detector_lock.py; update test_correctness_pass, test_data, test_canonical_name_examples, test_preprocess_papers_parallel_defaults, test_feature_port_parity, test_rust_from_dataset_contract, test_surname_hyphen_aware to drop sinonym. - docs/normalization_migration_blocked.md, docs/rust/arrow_dataset_spec.md: reword Sinonym references toward canonical normalization. No 'sinonym' references remain anywhere in the tree; Rust never carried a Sinonym path, so there is no Python<->Rust parity gap. Verified: ruff clean; full suite 1137 passed, 83 skipped, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… revert work_plan.md accumulates several updates from this session that share the file: - Arrow reader NULL-strictness revert rationale (NULL is the legitimate empty value; required_value would raise on real bundles). - Decided language-detection changes: fastText mandatory in production (load failure raises; testing off-switch retained); is_reliable requires both detectors to agree, non-agreement collapses to predicted_language='un'. - Corrected D6 empty-surname note: the Rust artifact-build path was a latent divergence, now mirrored and regression-tested. - Same-signature paper_author_list guard (Tier B) recorded as attempted then reverted / won't-fix (speculative guard; broke a pre-existing test). - Sinonym-removal doc cleanups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…(v5) Implements the two decided language-detection changes, Python and Rust in lockstep. #2 fastText mandatory in production: _get_fasttext_model raises on a genuine load failure instead of silently degrading to a cld2-only path. The testing off-switch (S2AND_SKIP_FASTTEXT / set_fasttext_loading_enabled) is retained. #4 is_reliable requires agreement: a pure reconcile_detected_languages (s2and.text and s2and_rust/src/language_detection.rs) returns is_reliable=True only when BOTH detectors return a concrete language AND agree; any non-agreement (disagreement, or only one detector responding) collapses to predicted_language='un', is_reliable=False, preserving the invariant is_reliable <=> language != 'un'. FEATURIZER_VERSION 4->5 (affects the is_reliable-sum, predicted_language equality, and english_or_unknown_count features). Tests: - reconcile_detected_languages truth-table unit tests in Python (tests/test_text.py) and Rust (language_detection.rs::reconcile_tests). - test_detect_language now asserts the disabled-fastText behavior (unreliable / 'un'); the failed-load test asserts the raise. - test_rust_from_dataset_contract: skip guard catches the raise; titles swapped to ones both detectors agree on (short titles collapse to 'un' as cld2 abstains), keeping the English-vs-German parity check. - test_eval_prod_models B3 (1.0, 0.816, 0.899) -> (1.0, 0.815, 0.898): the ~0.001 shift is the not-yet-retrained production model reacting to the new language features; regenerate at the v1.3 retrain. The production Arrow bundle stores Python-computed is_reliable/predicted_language columns read verbatim by Rust, so this only takes full effect once the bundle is regenerated (the v1.3 retrain does this). Verified: cargo test 34 passed; full suite 1141 passed, 83 skipped, 0 failed; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Arrow readers pin name-count last_first_initial keys to initial_char (raw_arrow_features.rs, rust_featurizer.rs) as a hardcoded assumption. A bundle built from a legacy_full_first_token ANDData would silently produce keys Rust reads with the wrong form and no downstream diagnostic -- a real risk since the v1.3 retrain regenerates the bundle. feature_block_from_anddata (the single ANDData->Arrow funnel) now raises if the source dataset declares a non- initial_char semantic, closing work_plan section-2 item #238. Negative test added; the existing mini-contract test covers the initial_char (pass) path. Verified: tests/test_feature_block.py from_anddata tests pass; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nsistent) work_plan section-2 #229 (subblocking ORCID gating asymmetry) is resolved as already-consistent, not a parity bug: a code trace shows both implementations gate ORCID on the per-request orcid_enabled flag, not field presence -- Rust raw_arrow_features.rs gates orcid_hash on orcid_enabled, Python query_adapter.mask_query_features sets orcid=None when disabled, and the subblocking repair pass uses the same use_orcid_subblocking flag in both. All field x flag combinations are consistent, so there is no contradictory combo to guard at runtime; the parity test is the guard. Adds test_orcid_enabled_false_suppresses_populated_orcid_field: with a populated, valid author_info_orcid, orcid_enabled=False suppresses ORCID at both the query-feature (extract_query_features) and cluster-summary layers -- pinning that the flag, not the field, is authoritative (guards against re-introducing a field-presence-only gate on the Python side). Verified: tests/test_query_adapter.py passes; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Closes #39 |
Address strict Rust prediction cleanup, Arrow training Specter loading, Rust featurizer cache invalidation, production model version warnings, CI no-sync behavior, transfer-mini compare ingest validation, and Rust LightGBM parser guards. Verification: uv run --no-sync ruff check/format on touched files; uv run --no-sync pytest focused regression set; uv run --no-project cargo test --manifest-path s2and_rust/Cargo.toml lightgbm_booster; uv run --no-project cargo fmt --manifest-path s2and_rust/Cargo.toml --check; git diff --check.
Reject null Specter embeddings, remove retired reference_features from archived scripts, align preprocess benchmark coauthor ngrams, warn on auto Rust prediction downgrade, centralize class-0 prediction, use dataclasses.replace for backend downgrade, and reuse shared Arrow validation helpers. Verification: uv run --no-sync ruff check on touched files; uv run --no-sync ruff format --check on touched files; uv run --no-sync pytest -q tests/test_arrow_training_ingestion.py tests/test_regression_fixes.py tests/test_predict_and_combine.py tests/test_incremental_linking_runtime.py; git diff --check.
The canonical_v2 cutover removed the _lasts_equivalent_for_constraint
shim and left the within-block last-name constraint comparing canonical
lasts exactly in Python, while Rust still compared space-insensitively —
a cross-language divergence, and the exact form would veto joined-vs-
spaced surname variants ("ouyang" vs "ou yang") that upstream blocking
deliberately groups under one block key.
Ruling: space-insensitive comparison is deliberate canonical_v2
compare-time policy (storage stays spaced per D5), not a legacy shim.
Adds s2and.text.canonical_lasts_equivalent, wires it into
ANDData.get_constraint, documents the Rust mirror, re-pins the
constraint tests, and records the ruling in the migration doc.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete the per-pair SQLite cache and use_cache plumbing; add s2and/feature_cache.py cached_featurize (per-split compressed NPZ, fail-closed content fingerprint over ingested source bytes, name-count binding, featurizer identity, and exact resolved pair lists; write-once under file lock). Clusterer config v3->v4 drops the use_cache key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Require pairwise_bundle_binding as an explicit argument on save_incremental_linking_artifact and metadata build; persist only the validated top-level field. New audit_metadata rejects the reserved key; legacy artifacts with an old nested audit copy load it as inert historical metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recover the approved v2->v3 unit from the phase2 stash: the generator counts dropped duplicate canonical pairs, and both the Python and Rust loaders require generation counts plus output cardinality to equal the input cardinality (v2 metadata left 3,768 of 9,925 input pairs unaccounted). Counts validate as unsigned 64-bit integers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recover the one-line tests/test_data.py hunk that belongs to the name-tuple v3 unit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete the one-off study (two CLIs, scripts/_pair_ablation/, docs/pair_ablation.md, five test files) per the 2026-07-20 owner decision. The complete study is preserved verbatim in the ignored scratch/pair_ablation_study/ workspace with a provenance README, and git history retains the tracked copies before this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recover the target-identity unit from the phase2 stash, adapted to the explicit pairwise-binding API: metadata schema v4 adds a required target_spec_digest (validated as SHA-256), save_incremental_linking_artifact requires the target spec, and both bundle finalization and production loading reject a target JSON whose canonical digest differs from the artifact — including a target modified after manifest checksums are refreshed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recover the fixture/config hardening hunks from the phase2 stash: pin fixture tolerances to a shared constant and reject relaxed values, require finite [0, 1] fixture probabilities, reject Boolean numeric config values, read bundle_status as a required manifest field, and validate promoted target JSON structure including duplicate features. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Derive the linker's expected pairwise binding from the manifest and clusterer config the load already validated instead of re-running pairwise_bundle_binding over the whole bundle, which re-hashed every declared file. Public pairwise_bundle_binding(path) keeps its validating boundary. A regression asserts one checksum per declared file on a normal complete-bundle load. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
IncrementalLinkingArtifactMetadata.__post_init__ already validates the contract on construction; the explicit call after build() re-validated the same object. The staged reload, atomic staging, fsync, locks, and manifest hashing are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add production-entrypoint tests showing predict_from_arrow_paths and predict_incremental_from_arrow_paths raise MissingArrowArtifactError when the raw-planner batch indexes are absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner decision 2026-07-20: keep the counts and index publishes as two sequential individually-atomic operations instead of a transactional pointer. A crash between them is fail-closed at consumption — the index manifest embeds the counts generation provenance and every consumer validates the four-field name-count binding — and rerunning generation with --overwrite repairs the tear. Add a real integration test that injects the crash, inspects the torn on-disk state, proves the binding rejects it, and proves the rerun repairs it. The stashed callback/rollback candidate stays unrecovered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete scripts/bench_python_name_counts.py and the in-file Rust benchmark harness that recreated the retired dense signature-index layout. The adaptive-layout rationale is recorded in docs/rust/runtime.md; the real layout-selection unit tests remain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove model._missing_arrow_prediction_artifacts_error, production._summarize_query_views / promoted_incremental_observed_probe / _raw_arrow_plan_window_size / _raw_arrow_plan_windows, policy.arrow_paths_have_name_counts_index, subblocking._load_canonical_orcid_prefix_counts, and ANDData._compute_signature_name_counts. Useful assertions moved to live paths first: ORCID loader tests go through _LazyCanonicalOrcidPrefixCounts.load(), and canonical count-key coverage runs through the batched preprocess_signatures count path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the six dead serde derives with their field attributes and unused imports (compiler-verified), drop the docstring reference to the gitignored canonical-example generator, remove the stale optional comment on the enabled aarch64 matrix entry, and rewrite the release-workflow policy tests to assert on parsed YAML structure instead of raw-text regex and string counts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
No description provided.