lance 9 / lancedb 0.33 / DataFusion 54 / Rust 1.97.1 — measured, then landed - #896
Merged
Conversation
…-> =0.33.0, datafusion 53 -> 54 Operator-requested what-breaks assessment; NOT a migration and NOT verified. Compilation is still in flight at commit time - do not merge on the strength of this commit alone. Findings so far, variables isolated: - Rust 1.97.1 on the CURRENT pins is clean: contract passes clippy --all-targets -D warnings; the only reds are the 12 pre-existing lance-graph-ontology lints already on the board. - Resolution with lance 9 SUCCEEDS: lance 9.0.0 / lancedb 0.33.0 / datafusion 54.1.0 / arrow 58.3.0 / object_store 0.13.2, no conflict and no unused-patch warning. arrow and object_store do NOT move - only datafusion steps a major. A stale datafusion 53.1.0 also stays in the graph (duplicate major, source not yet identified). - Version correction: there is no lancedb 0.34-0.36 Rust crate. The registry tops out at 0.33.0, which is itself the lance-9 pairing (it pins lance =9.0.0). 0.36 is the PyPI package, versioned independently. - Policy finding (unresolved, NOT acted on): AdaWorldAPI forks exist for both lance and lancedb, but the workspace depends on the registry. Both forks are stale (no tags, main only) so neither can supply v9 today. Per P0, unknown fork coordinates means stop and ask - flagged, not changed.
…e ruling Operator ruling E-LANCE-IS-UPSTREAM-AUTHORITATIVE-1: lance/lancedb and the whole lance-* family are consumed from crates.io upstream, NEVER from a fork - too risky, upstream is authoritative. This CORRECTS the CLAUDE.md P0, which had explicitly named those crates in its must-be-wired-via-the-fork list; the P0 is regraded in place with a carve-out (append-only; the rule still stands for ndarray, surrealdb and every other forked crate). The trap it closes: AdaWorldAPI/lance and AdaWorldAPI/lancedb DO exist, and I read their existence plus the P0's naming as a standing violation to fix - it is the opposite. A fork's existence is not evidence of intent to depend on it. First real API break found, and it is DataFusion 54, not Lance 9: DataFusion 54 made Any a SUPERTRAIT of TableSource instead of a method on it, so the explicit as_any impl in lance-graph-catalog is now E0407. Fixed by deleting the method (downcasting goes through the blanket Any impl) with a comment naming the cause. Everything upstream of that crate had already compiled clean under lance 9: lance 9.0.0 itself, lance-index, lance-tokenizer, lance-datagen, lance-geo, lance-index-core, datafusion 54.1.0, plus our lance-graph-planner, lance-graph-cognitive, holograph and bgz-tensor.
DF 54 moved Any from a METHOD to a SUPERTRAIT on TableSource, TableProvider and ScalarUDFImpl alike, so every explicit as_any impl is E0407. Verified per trait against the 54.1.0 sources before deleting - not a blanket assumption: TableSource: trait TableSource: Any + Sync + Send TableProvider: trait TableProvider: Any + Debug + Sync + Send ScalarUDFImpl: trait ScalarUDFImpl: Debug + DynEq + DynHash + Send + Sync + Any Downcasting now goes through the blanket Any impl; no call site changes. Recorded while here, NOT yet acted on: ScalarUDFImpl in 54 also requires DynEq + DynHash, which are blanket-implemented for T: Eq + Any and T: Hash + Any. So all 11 UDF types must become Eq + Hash. DF 54's Signature is itself derive(PartialEq, Eq, Hash), so signature-only UDFs can derive - but the ones carrying func: Arc<dyn Fn(&[ColumnarValue]) -> Result<ColumnarValue>> (and NsmSimilarityUdf's Arc<NsmRuntime>) CANNOT: a closure trait-object has neither equality nor a hash. Those need hand-written impls keyed on identity fields, which is a semantic decision about what makes two UDFs the same expression - deferred to the report, not guessed here.
Operator-directed: bump now, fix after. rust-toolchain.toml 1.95.0 ->
1.97.1, and the three Dockerfiles with it - they were installing
1.94.0, a pre-existing drift BEHIND the repo's own 1.95.0 pin, so they
were two versions stale rather than one. (ruff's Dockerfile has the
drift-proof pattern: COPY rust-toolchain.toml + --default-toolchain
none, so it can never disagree with the pin. Worth adopting here.)
Toolchain measured clean on the OLD pins first, deliberately, so a
toolchain failure could never be mistaken for a dependency failure:
lance-graph-contract passes clippy --all-targets -D warnings; the only
reds are the 12 pre-existing lance-graph-ontology lints already on the
board. NOTE this bump does NOT satisfy rust-toolchain.toml's own
documented rule ('bump explicitly when a future version is reviewed
and workspace clippy passes clean') - workspace clippy is not clean
because of those pre-existing lints. Bumping ahead of that gate is the
operator's explicit call; the fix-after is owed.
Also adds the probe plan and 4 invariant comments on the UDF
PartialEq impls (no behaviour change). The name()-keyed equality is
CORRECT for DataFusion - a UDF's identity in expression comparison and
CSE is its registered name, not its payload - but at three sites it
ASSERTS rather than restates: CamDistanceUDF's closure captures a
caller-supplied codebook and NsmSimilarityUdf's an Arc<NsmRuntime>,
neither compared, so same-name => same-behaviour is load-bearing and
was invisible. Now documented with the one-per-session-context rule.
VectorDistance/Similarity are safe by construction (func derives from
the compared metric) and say so.
…gs clean These are what made 'workspace clippy passes clean' - rust-toolchain.toml's own precondition for a toolchain bump - unsatisfiable, so clearing them is the first owed fix-after of the 1.97.1 bump. Pre-existing, NOT 1.97 arrivals (they fire on 1.95 identically). 5x use of deprecated oxrdf::Subject -> oxrdf::NamedOrBlankNode. Verified semantically exact before substituting: oxrdf 0.3.3 declares 'pub type Subject = NamedOrBlankNode' behind #[deprecated], and NamedOrBlankNode carries the identical NamedNode/BlankNode variants. A type-alias rename, no behaviour change. 6x doc list item indentation, in two directions - worth recording because the naive fix oscillates between the twin lints. clippy's 'try using 4 spaces' is NOT the fix for a list-item CONTINUATION: 4 spaces flips 'overindented' into its twin 'without indentation'. A continuation must align with the list item's TEXT column (marker '- ' at 3 spaces => text at 5), so the sweep computes the column from the nearest preceding item rather than applying a constant. One of the six was NOT a continuation at all: l15.rs's two trailing lines are standalone prose after the list, and indenting them (which silences clippy) would have made them read as part of the account.tax bullet. Fixed by ending the list with a blank doc line - clippy's own second suggestion - preserving the meaning instead of the appearance.
…-existing lints Second owed fix-after of the toolchain+lance-9 bump. Every crate now passes clippy --all-targets -D warnings: lance-graph, -catalog, -contract, -ontology, -planner, -supervisor, cognitive-shader-driver, and -callcenter under BOTH its query and query-lite features (the default-feature check was a false green - vsa_udfs and ontology_table are feature-gated, so the first pass never compiled them). Follow-ons from my own as_any sweep: - savant_reasoners.rs: 4x manual_async_fn (the impls wrapped an async block in a hand-written impl Future; RPITIT lets the impl just be async fn). Dropped the now-unused core::future::Future import, and moved savant_by_name into the TEST module - it is test-only, so a module-scope import made it unused in the lib build. - ontology_table.rs: std::any::Any unused once as_any was deleted. Pre-existing, never gated because the feature was never in the gate: - vsa_udfs.rs: sort_unstable_by -> sort_unstable_by_key(Reverse) (the exact same comparator; tie order stays implementation-defined either way), 2x needless_range_loop -> enumerate() with a debug_assert that the collection length matches the arrow array's. - zerocopy.rs: 2x iter_kv_map, 1x unnecessary_cast (clippy --fix). - bindspace.rs push_typed: too_many_arguments 8/7 -> #[expect] with a reason. One argument per SoA column IS the point; folding them into a params struct would re-wrap the axes in a new struct, the exact anti-pattern the AGI-as-SoA invariant forbids. - driver.rs/engine_bridge.rs/end_to_end.rs: needless_range_loop where the index IS the data (position-coupled bit patterns across two plane rows) -> #[expect] with reasons, not rewrites; a constant assert! -> const _: () = assert!(...); a constant-folded bit expression spelled through a binding. - ndarray_bridge.rs: 8 SIMD/scalar fallbacks were dead under the default ndarray-hpc feature - gated to cfg(not(feature = "ndarray-hpc")) alongside the dispatch arms that call them, and their two tests with them. - Two examples: a doc line starting with '+' read as a markdown bullet; a range loop over facts -> iter().enumerate().
…, agent log Records the measured assessment turned landed bump (9 repos, 8 sibling PRs merged), both operator rulings, and two things worth a future session's time: - The false green. A default-feature clippy pass on lance-graph-callcenter reported EXIT=0 while never compiling vsa_udfs or ontology_table - both feature-gated, and both carrying the very as_any impls the DF-54 break hits. Only explicit impls break, and only when their module actually compiles, so a gated module looks fine indefinitely. The fix was gate-aware gating, not more trust in an exit code. - My falsified prediction, kept rather than quietly dropped: I read DF 54's DynEq + DynHash supertraits as a NEW requirement without diffing 53, which already declared them. The tree already satisfied it. The claim reached a commit message before measurement corrected it; the lesson (diff the versions, do not read the new one) is in the plan. Also records that both Sonnet workers stopped on their explicit disk floor and correctly blamed an external target dir rather than their own residue - the argument for giving grindworkers a numeric floor and permission to halt.
|
Important Review skippedToo many files! This PR contains 121 files, which is 21 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (121)
You can disable this status message by setting the ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bdc0e256-f835-455a-aa92-4ebb1e51807c) |
AdaWorldAPI
marked this pull request as ready for review
August 5, 2026 15:53
CI clippy went red on ONE error, and it corrects something I said
earlier in this arc: I reported 1.97.1 as adding "no new lint surface".
It does — clippy::some_filter is new in 1.97, and this is the proof. My
claim was true only of the crates I had scoped, which is a narrower
statement than the one I made.
Why the -p sweep never saw it: deepnsm is workspace-EXCLUDED, so no
-p run reaches it; CI checks it by --manifest-path. Reading the CI
config rather than guessing also showed the gate is TIERED —
lance-graph-contract, deepnsm and deepnsm-v2 are MANDATORY while
lance-graph and bgz-tensor are advisory — so only the mandatory set had
to move. Verified all three mandatory manifests: 0 errors; deepnsm
tests 217 + 4 + 8 + 2 green.
The fix is simpler than clippy's own suggestion and equivalent:
animate.or(Some(false).filter(|_| animate.is_none()))
is just animate.or(Some(false)). Option::or already discards its
argument when self is Some, so the filter's predicate could only ever be
true on the None branch — where the filter is a no-op. Clippy proposed
animate.is_none().then_some(false), which is also correct but keeps a
condition that cannot change the result.
Recorded, NOT fixed (deliberately): the ungated/advisory excluded crates
carry pre-existing clippy debt — bgz17 27, lance-graph-codec-research
55, bgz-tensor 10 (advisory) — mostly needless_range_loop. CI does not
gate bgz17 or codec-research at all, and the workflow's own comments
already describe ~91 pre-existing lance-graph violations as debt to be
paid down over time. Fixing 92 unrelated lints inside a dependency-bump
PR would bury the bump; they belong in their own pass.
AdaWorldAPI
force-pushed
the
claude/x265-x266-plans-review-h9osnl
branch
from
August 5, 2026 15:56
c7a26f4 to
626bb28
Compare
All 10 reported errors were ONE cluster - a single clippy::doc_lazy_continuation paragraph in adaptive_codec.rs that clippy counted nine times. A blank /// line ending the doc list cleared every one of them, so the raw error count overstated the work by ~9x. Same twin-lint family that recurred in lance-graph-ontology: prose following a doc list needs a blank line to END the list, never indentation. Nothing touched arithmetic, quantization, table indexing or iteration order - which matters here, because this crate's whole premise is that a table lookup reproduces a computed distance. No #[expect] was needed and no SAFETY comment moved. Re-verified centrally: 0 clippy errors, 207 tests pass. fmt also reflowed two files (line-wrapping only). Committed alone while two sibling agents are still live-editing crates/bgz17 and crates/lance-graph-codec-research - staging their half-finished edits would land a broken intermediate state.
~30 errors → 0 across 15 files. Dominated by needless_range_loop (18 of them), plus 3 unused imports, a non_snake_case test fn, assertions_on_constants, manual_div_ceil, identity_op, unused_variables, and one doc_lazy_continuation the survey had missed. The correctness fence held where it mattered: every needless_range_loop fix used enumerate() with the index still bound where the body needs it, so index ORDER is preserved and no palette/SIMD kernel semantics moved. No public API changed — PaletteMatrix, PaletteCsr, Base17, batch_palette_distance, TypedPaletteGraph and the container pack/unpack surface are consumed by other crates, so a signature change would have broken callers. No unsafe block touched, so no SAFETY comment moved. Re-verified centrally rather than on the worker's word: 0 clippy errors, 134 tests pass (the crate has grown past the ~121 in its docs). A real defect the lints exposed, recorded NOT fixed: in src/palette.rs::test_sigma_band_no_empty the inner loop's variable is unused in the body, so the nested loop re-asserts the same i-only condition instead of adding coverage. That is a vacuous-assertion instance of exactly the kind CLAUDE.md's falsifiability rule names — it is pre-existing and rewriting test logic was out of scope for a lint sweep, but it wants a real second axis, not an enumerate().
…-verified) 20 clippy errors -> 0 across eight files: needless_range_loop (the bulk -- iter()/enumerate()/zip() where the index only subscripted), unused imports pulled out of the module preamble or moved into `mod tests`, manual_div_ceil, manual_range_contains, len()>=1, a Default impl beside the inherent `new`, and three genuinely dead accumulators deleted (`confident_count`, `component_energy`, `prev_bands`, `pal_scents` -- each declared next to siblings that ARE consumed, which is what made them read as live). One `#[expect(clippy::needless_range_loop)]` in `BasePalette::distance_matrix` with a reason: the symmetric `mat[i][j]` / `mat[j][i]` double-write needs two positions in one `Vec<Vec<u32>>` at once, which no single iterator expresses without split_at_mut gymnastics. `decode_crystallized` became a three-way zip rather than an index because it walks three parallel BARK_BANDS arrays -- zip makes the band-for-band correspondence structural instead of trusting three subscripts to agree. TESTS ARE NOT GREEN AND THAT PREDATES THIS COMMIT. 56 pass, 9 fail, all nine panicking at the same `transform.rs:46` MDCT length assert. Verified by stashing this entire diff and re-running at HEAD: identical 56/9, identical test names. The diagnosis (the assert demands 4N input for an N-coefficient call that every caller feeds 2N) and the reason nobody caught it (the crate is workspace-excluded AND ungated at both CI tiers) are recorded as ISS-CODEC-RESEARCH-MDCT-ASSERT. Fixing it is a research-codec correctness question, not a lint question, and it does not belong in a lint commit.
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
2 participants
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 this is
The lance-9 / DataFusion-54 / Rust-1.97.1 bump for lance-graph, plus the measured assessment behind it. Sibling repos already merged the same sweep: OGAR #244/#245, ruff #93, stockfish-rs #14, woa-rs #179, a2ui-rs #19, MedCare-rs #351.
Version correction, first
There is no
lancedb0.34–0.36 Rust crate. The registry tops out at 0.33.0, and that release is the lance-9 pairing (it pinslance = "=9.0.0"exactly).0.36.0is the PyPI package, versioned independently of the Rust crate.Method
The toolchain was tested alone, on the old dependency pins, first — so a toolchain failure could never be mistaken for a dependency failure. It came back clean (no new lint or language surface), which made every subsequent red attributable to the dep bump.
target/was cleared between legs since a rustc-version swap invalidates every artifact anyway.What actually breaks: one uniform DataFusion change
DF 54 moved
Anyfrom a method to a supertrait on seven traits at once — verified per trait against the 54.1.0 sources, not assumed:TableSource·TableProvider·ScalarUDFImpl·CatalogProvider·SchemaProvider·AggregateUDFImpl·WindowUDFImplCost: 12
as_anydeletions, zero call-site changes (.as_any()still resolves through the blanketAnyimpl). Zerofn as_anynow remain incrates/— a text-level guarantee stronger than any per-feature compile.arrow stays 58 and
object_storestays 0.13.2. Historically the two that force painful cascades; they don't move. Only DataFusion crosses a major, and after bumping the two workspacedatafusionpins all 20lance-*crates resolve to a single major. The residualdatafusion 53.1.0comes from exactly one source —deltalake-core, behind the non-defaultdeltafeature — so it costs a default build nothing.A false green worth knowing about
clippy -p lance-graph-callcenterreturned EXIT=0 while never compilingvsa_udfsortranscode/ontology_table— both feature-gated, and both carryingas_anyimpls the break hits. Only explicit impls break, and only when their module actually compiles, so a gated module can look fine indefinitely. Re-run under--features queryand--features query-lite, both now green. A parallel audit had concludedTableProviderwas untouched by this change; it isn't (table.rs:54in 53 has the method, 54 doesn't).A prediction of mine that measurement falsified
I predicted a second migration: all ~11 UDF types needing
Eq + Hash, since DF 54'sScalarUDFImpllistsDynEq + DynHashsupertraits blanket-implemented overEq + Any/Hash + Any. Wrong — DF 53 already required them (datafusion-expr-53.1.0/src/udf.rs:498), so the tree already satisfied it with hand-writtenname()-keyed impls at every site. I read the new version's bound and inferred "new requirement" without diffing the old one. Kept in the plan (§4) rather than quietly dropped, because the lesson generalizes.What did land there: invariant comments on 4
PartialEqimpls, no behaviour change.name()-keyed equality is correct for DataFusion (a UDF's identity in expression comparison and CSE is its registered name), but at three sites it asserts rather than restates —CamDistanceUDF's closure captures a caller-supplied codebook andNsmSimilarityUdf's anArc<NsmRuntime>, neither compared. That assumption was invisible; now it's documented with the one-per-session-context rule.Contents
.claude/plans/lance9-datafusion54-upgrade-probe-v1.md— the assessment, with the falsified prediction and the honest still-open list.rust-toolchain.toml+ 3 Dockerfiles (which had drifted to 1.94.0, behind the repo's own pin —ruff's Dockerfile has the drift-proof pattern worth copying).-D warnings:lance-graph,-catalog,-contract,-ontology,-planner,-supervisor,cognitive-shader-driver,-callcenter(both features). The 12 long-standinglance-graph-ontologylints are cleared — that's what maderust-toolchain.toml's own "bump when clippy is clean" precondition satisfiable rather than aspirational.LATEST_STATEhead,AGENT_LOGrun entry,E-LANCE-IS-UPSTREAM-AUTHORITATIVE-1.Owed, not claimed done
OGAR's 22 unswept crates + one unverified fix (worker running);
blockly-rs/rigstill on 1.95.0;-benches/symbiont/cognitive-stack/surreal_container/-pythonunchecked; thedeltafeature still broken by deltalake 0.32's own drift.🤖 Generated with Claude Code
https://claude.ai/code/session_01K3RyLEbuNSHxxB3NTTrGki
Generated by Claude Code