Skip to content

Commit d5b3858

Browse files
lmeyerovclaude
andcommitted
fix(lint): turn master green by removing the 7 casts over the type-hygiene baseline
The per-file ratchet in bin/ci_type_hygiene_baseline.json is a SNAPSHOT, so it goes stale whenever a PR other than the one that owns it touches a baselined file. #1830 captured its baseline on a branch whose merge-base is b6181d3 -- before #1800, #1799 and #1816 landed -- and each of those three added cast() calls to a file #1830 had already pinned. All four were green on their own bases; the merged combination was not. The baseline is UNCHANGED. The findings are removed instead: - 3 of the 7 were never typing.cast. The guard matches any call named `cast`, including the attribute form, so pl.Expr.cast -- a polars RUNTIME dtype conversion -- counts as a typing finding. Those carry the documented `# hygiene-ok: explicit-cast` escape hatch with a reason. - The other 4 are real and are gone by DECLARATION rather than by call-site assertion. _two_hop_cached_equal_domain_degree_counts declares `counts: Tuple[DataFrameT, DataFrameT]` once, collapsing four casts into one localized `# type: ignore[assignment]` on the polars arm. _apply_connected_optional_match declares `seed_ids: SeriesT` / `node_ids: SeriesT`, since selecting one column off a frame is a Series on every engine. All three files now sit at or below baseline (18/18, 132/133, 41/42). Typing-only: typing.cast is the identity function at runtime, so every removal is provably value-preserving. The one restructured line binds an unchanged pl.Expr list to a name so the per-line suppression fits the 127-column limit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
1 parent 6863879 commit d5b3858

4 files changed

Lines changed: 22 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
3737
- **GFQL execution context is declared rather than attached at runtime**: the private `_gfql_*` per-execution fields (index policy and registry, row-pipeline base graph, carried seed nodes, edge aliases, shortest-path backend, and the indexed-bindings handoff) are now declared on `Plottable` with defaults on `PlotterBase`, instead of being set with `setattr` and read back with `getattr(..., default)`. No public API or behaviour change; internal call sites are typed, and hand-rolled `Plottable` stand-ins must now construct the full context.
3838

3939
### Fixed
40+
- **`master` is green again: the type-hygiene ratchet tripped on merge, and the fix is fewer `cast()`s rather than a raised cap**: `bin/ci_type_hygiene_baseline.json` is a per-file SNAPSHOT, so it goes stale whenever a PR *other than the one that owns the baseline* touches a baselined file. #1830 captured its baseline on a branch whose merge-base is `b6181d355` — before #1800, #1799 and #1816 landed — and each of those three added `cast()` calls to a file #1830 had already pinned. All four were green on their own bases; the combination was not, and nothing in any of the four merges signalled it: `explicit-cast` came out 7 over baseline across `gfql_fast_paths.py` (+5), `gfql_unified.py` (+1) and the polars `row_pipeline.py` (+1), failing `python-lint-types` on 3.11/3.13/3.14. The baseline is **unchanged** here; the findings were removed instead. Three of the seven were never `typing.cast` at all — the guard matches any call whose name is `cast`, including the attribute form, so `pl.Expr.cast`, a polars RUNTIME dtype conversion, counts as a typing finding; those three carry the documented `# hygiene-ok: explicit-cast` escape hatch with the reason. The other four were real, and are gone by DECLARATION rather than by call-site assertion, which is the idiom the guard is asking for: `_two_hop_cached_equal_domain_degree_counts` declares `counts: Tuple[DataFrameT, DataFrameT]` once and both arms assign to it, so four casts collapse to one localized `# type: ignore[assignment]` on the polars arm (`DataFrameT` is pinned to pandas at checking time); `_apply_connected_optional_match` declares `seed_ids: SeriesT` / `node_ids: SeriesT` instead of casting, because selecting one column off a frame is a Series on every engine. (The neighbouring `cast(DataFrameT, df_to_engine(...))` pair is deliberately left alone: `df_to_engine` carries no return annotation, so removing those would mean annotating a helper with 115 call sites, and the polars arm of that branch is not reached by any CI lane — rewriting it would have dragged a pre-existing coverage blind spot into the changed-line gate for no typing gain.) All three files now sit AT or BELOW baseline (18/18, 132/133, 41/42). Typing-only: `typing.cast` is the identity function at runtime, so every removal is provably value-preserving, and the one restructured line binds an unchanged `pl.Expr` list to a name so the per-line suppression fits the 127-column limit. **The hazard is latent, not spent** — any future PR adding a finding to a baselined file reds `master` the same way, and the guard cannot see it from inside either PR.
4041
- **`sum()`/`avg()` over a non-numeric column now raise the same typed error on every engine — polars no longer answers `avg(<string>)` with a silent `null`, and pandas no longer answers `sum(<string>)` with the string CONCATENATION**: the aggregate kernels are written four times (pandas/cuDF row pipeline, native polars row pipeline, the OLAP single-hop grouped fast path, and the fused lazy lane #1823 added in front of it) and each had inherited its host library's opinion about non-numeric input, so the SAME query answered differently depending on the engine: `avg(n.name)` raised on pandas but returned `null` on polars, while `sum(n.name)` returned `'abac'` on pandas and leaked a raw `polars.exceptions.InvalidOperationError` through the GFQL surface. "Match the other engine" was not available, because the two engines were wrong in OPPOSITE directions — so the contract is pinned to Cypher instead. openCypher/Neo4j declares `avg(input)` and `sum(input)` over `INTEGER | FLOAT | DURATION` only (neo4j/docs-cypher `functions/aggregating.adoc`) and enforces it: Neo4j 5.26.26 answers `RETURN avg(r.s)` over strings with *"AVG(...) can only handle numerical values, duration, or null."* and `sum(date(...))` with *"Type mismatch: expected Float, Integer or Duration but was Date"*; Kuzu 0.11.3 rejects both at bind time. A non-numeric input is therefore a **`GFQLTypeError` (E302) naming the aggregate and the user's output alias**, on every engine and on both the row pipeline and the fast path. **This is a deliberate contract change on the pandas engine**: `sum(<string column>)` used to return the concatenation, which is a silent wrong answer with no meaning in Cypher, and it was already inconsistent with `avg()`, which raised. Also brought to the same contract, all found by a full aggregate × dtype differential sweep rather than one at a time: `sum` over temporal and categorical columns (polars returned `null`, pandas raised), `sum`/`avg` over an all-null column (now `0` / `null` per Neo4j's *"`sum(null)` returns `0`"*, substituted rather than delegated — the host kernels answer that case `0`/`''`/`NaT`/`TypeError` depending on dtype, and polars raised), and `min`/`max`/`collect`/`count(DISTINCT)` over a **categorical** column, which Cypher accepts as `ANY` but which pandas rejected outright and cuDF answered with a *category label instead of a count*. A 180-cell polars × cuDF × polars-gpu matrix over 6 aggregates × 10 dtype columns (`_MATRIX_AGGS` × `_MATRIX_COLS`, each compared against the pandas oracle on BOTH the value and the error class) now shows **zero divergences**. Error-path only: the guards read a dtype and do not touch value computation on any served shape.
4142
- **A raw `polars.exceptions.*` could reach the caller from the native polars row pipeline**: on the pandas/cuDF surface `execute_call` wraps any kernel exception as `GFQLTypeError(E303)`, but the native polars path runs BEFORE `execute_call` and so skipped that wrapper entirely — `polars.exceptions.InvalidOperationError: `sum` operation not supported for dtype `str`` was reaching users verbatim, third-party class and all. Native row ops now funnel polars errors through the same wrapper with the same code and message shape, preserving the polars text as the exception cause.
4243
- **Three BOUNDED variable-length shapes returned a silently different count on `engine='polars'` (#1787)**: the native polars `rows(binding_ops=...)` builder rebuilds a variable-length segment from the raw matching edge table, and for three bounded shapes that rebuild produces a different edge multiplicity than the pandas oracle — with no error, just a different number. They now raise `NotImplementedError` like every other shape this lowering cannot reproduce, which is a deliberate behaviour change: these were *served* before, so a silent wrong answer becomes a loud, actionable error, and `engine='pandas'` still answers all of them. The three: (1) directed `-[*k..m]->` with `min_hops >= 3`, and `min_hops >= 2` when the segment starts from a filtered seed — `max_reached_hop` in `compute/hop.py` is a dedup-by-node BFS eccentricity rather than a longest-walk length, so the oracle prunes to empty where the rebuild expands a different edge multiset; (2) the DEGENERATE undirected window `-[*1..1]-` / `-[*1]-`, which halved the count — it resolves to `min == max == 1` and so is not "multihop", yet pandas still routes it through the variable-length hop, which is exactly why it slipped past the existing gates (the gate therefore keys on an explicit variable-length window, not on `is_multihop`); (3) undirected `-[*1..k]-` that does not start from the full node set (filtered seed, or a non-first segment), where the doubled-pair expansion over-counts. Every neighbouring shape stays native and is pinned as such — unseeded `min_hops <= 2`, the directed twin of each undirected decline, the directed degenerate window, and the plain `-[]-` edge — so the gate is a scalpel rather than a blanket refusal of variable-length segments. Same root-cause family as the unbounded shapes #1781 declined; the gate should shrink again once the multiplicity is reconstructible. Found by differential fuzzing against the pandas oracle, and pinned by an engine-parametrized suite (pandas / polars / cuDF / polars-gpu) that encodes the intended per-engine behaviour — the polars engines must decline exactly where the pandas-API engines must answer — plus a seeded fuzz that fails if the gate declines too much.

graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ def _agg_expr(func: str, expr: Optional[str], columns: Sequence[str], alias: str
878878
# (`categorical.rs: not implemented`), which escapes as a pyo3 PanicException -- not even
879879
# a polars exception, so nothing on the python side can wrap it. Casting to String makes
880880
# the aggregate well-defined on every polars version AND matches what pandas returns.
881-
col = col.cast(pl.String)
881+
col = col.cast(pl.String) # hygiene-ok: explicit-cast -- pl.Expr.cast is a runtime dtype conversion, not typing.cast
882882
# pandas aggs skip NaN (skipna); polars skips only NULL and treats NaN as a value (NaN == NaN
883883
# is True, so self-inequality can't detect it). For FLOAT columns convert in-query NaN -> null
884884
# first so every agg matches the oracle (pandas sum([nan, 1]) == 1 vs raw polars == nan).

graphistry/compute/gfql_fast_paths.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,11 @@ def _two_hop_cached_equal_domain_degree_counts(
526526
if cache is not None and full_key in cache:
527527
return cast(Tuple[DataFrameT, DataFrameT], cache[full_key])
528528

529+
# Declared ONCE so neither arm needs a call-site ``cast``: the polars arm produces
530+
# ``pl.DataFrame`` and the pandas/cuDF arm produces ``pd.DataFrame``, and ``DataFrameT`` is
531+
# pinned to pandas at checking time (graphistry/compute/typing.py). One localized ignore on
532+
# the polars assignment replaces four casts; the values are untouched either way.
533+
counts: Tuple[DataFrameT, DataFrameT]
529534
if engine in POLARS_ENGINES:
530535
import polars as pl
531536
# MEMO-MISS lane: ONE lazy plan for both degree arms. Eagerly this materialized the
@@ -543,13 +548,13 @@ def _two_hop_cached_equal_domain_degree_counts(
543548
filtered_edges.group_by(dst_col).len("__in_count__"),
544549
filtered_edges.group_by(src_col).len("__out_count__"),
545550
])
546-
counts = (cast(DataFrameT, in_counts), cast(DataFrameT, out_counts))
551+
counts = (in_counts, out_counts) # type: ignore[assignment] # polars frames; DataFrameT pins pandas
547552
else:
548553
domain_ids = domain_nodes[node_col].drop_duplicates()
549554
filtered_edges = edge_domain[edge_domain[src_col].isin(domain_ids) & edge_domain[dst_col].isin(domain_ids)]
550555
counts = (
551-
cast(DataFrameT, filtered_edges.groupby(dst_col, sort=False).size().reset_index(name="__in_count__")),
552-
cast(DataFrameT, filtered_edges.groupby(src_col, sort=False).size().reset_index(name="__out_count__")),
556+
filtered_edges.groupby(dst_col, sort=False).size().reset_index(name="__in_count__"),
557+
filtered_edges.groupby(src_col, sort=False).size().reset_index(name="__out_count__"),
553558
)
554559

555560
if cache is not None:
@@ -1670,7 +1675,10 @@ def ids_of(frame: DataFrameT) -> "pl.LazyFrame":
16701675
in_counts
16711676
.join(out_counts, left_on=dst_col, right_on=src_col, how="inner")
16721677
.select(
1673-
(pl.col(_TWO_HOP_IN_COUNT_COL) * pl.col(_TWO_HOP_OUT_COUNT_COL))
1678+
# `.cast(pl.Int64)` below is `pl.Expr.cast` -- a polars RUNTIME dtype conversion,
1679+
# not `typing.cast`. The hygiene guard matches any call named `cast`, so the
1680+
# suppression rides the line the guard reports (the head of the chained call).
1681+
(pl.col(_TWO_HOP_IN_COUNT_COL) * pl.col(_TWO_HOP_OUT_COUNT_COL)) # hygiene-ok: explicit-cast -- polars dtype cast
16741682
.sum().fill_null(0).cast(pl.Int64).alias(alias)
16751683
)
16761684
)
@@ -2222,7 +2230,10 @@ def join_props_polars(work_df: Any, alias: str, node_df: Any, edge_col: str) ->
22222230
from graphistry.compute.gfql.lazy.engine.polars.dtypes import is_stringlike
22232231
cat_cols = [c for c, dt in work.schema.items() if is_stringlike(dt) and dt != pl.String]
22242232
if cat_cols:
2225-
work = work.with_columns([pl.col(c).cast(pl.String) for c in cat_cols])
2233+
# `pl.Expr.cast` is a polars RUNTIME dtype conversion, not `typing.cast`; bound to a
2234+
# name so the guard's per-line suppression fits inside the 127-col limit.
2235+
to_string = [pl.col(c).cast(pl.String) for c in cat_cols] # hygiene-ok: explicit-cast -- polars dtype cast
2236+
work = work.with_columns(to_string)
22262237
work_schema = work.schema
22272238
for alias, func, expr_alias in aggregations:
22282239
# Same Cypher sum()/avg() type contract the row-pipeline kernels enforce (see

graphistry/compute/gfql_unified.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,10 @@ def _optional_arm_start_nodes(
411411
else:
412412
seed_frame = cast(DataFrameT, df_to_engine(
413413
seed_src.dropna().drop_duplicates().rename(columns={joined_col: node_col}), concrete_engine))
414-
seed_ids = cast(SeriesT, seed_frame[node_col])
415-
node_ids = cast(SeriesT, base_nodes[node_col])
414+
# Declared, not cast: selecting one column off a frame is a Series on every engine, so
415+
# the annotation states that directly instead of re-asserting it at the call site.
416+
seed_ids: SeriesT = seed_frame[node_col]
417+
node_ids: SeriesT = base_nodes[node_col]
416418
if is_polars_df(base_nodes):
417419
return cast(DataFrameT, base_nodes.filter(node_ids.is_in(seed_ids)))
418420
return cast(DataFrameT, base_nodes[node_ids.isin(seed_ids)].copy())

0 commit comments

Comments
 (0)