diff --git a/CHANGELOG.md b/CHANGELOG.md index 67bde7a5e9..0066ed15e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **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. ### Fixed +- **`show_indexes()` reported `valid=True` for an index the resolved query engine cannot use**: GFQL physical indexes are engine-specific, so a fingerprint-fresh index built for another engine (e.g. a polars-built index inspected on a graph whose AUTO queries resolve to pandas) silently declines to a scan at query time — yet `show_indexes()` told the user everything was fine. This is the follow-up proposed in #1767's disposition ("stop `show_indexes()` reporting `valid=True` for an index the resolved query engine cannot use"); #1838 made the decline loud in `gfql_explain`, and this completes the pair on the inspection surface. `valid` keeps its fingerprint-only meaning (backward compatible); three columns are added: `query_engine` (what the `engine` argument — default `'auto'` — resolves to for THIS graph, the same resolution a query makes), `usable` (fresh AND engine-matched), and `reason` (why not, reusing #1838's mismatch wording, e.g. `resident edge_out_adj index engine=polars, requested engine=pandas -> scan`, plus a stale-fingerprint reason). `show_indexes(engine=...)` previews an explicit engine choice, and `SHOW GFQL INDEXES` respects the `gfql(engine=...)` of the call. Reporting-only: no query routing changes (#1743 AUTO-routing remains on hold). - **The compiled-query cache key omitted the engine, which a shared cache would have turned into a wrong answer.** The key relied on `node_dtypes` to capture engine-dependent compilation, but polars and polars-gpu report *identical* node dtypes and compile differently. While the cache was partitioned per `Plottable` this was masked rather than avoided; sharing plans process-wide exposed it immediately as 16 failures in `test_const_fold_engine_parity.py` on the polars-gpu arm. The key now carries the **resolved** engine -- resolved, because `auto` resolves per graph, so keying on the requested engine would let two graphs that resolve differently share a plan. - **`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. diff --git a/docs/source/gfql/indexing.rst b/docs/source/gfql/indexing.rst index d63dc535db..c25a28ba40 100644 --- a/docs/source/gfql/indexing.rst +++ b/docs/source/gfql/indexing.rst @@ -100,7 +100,7 @@ API): g = g.gfql_index_edges("forward") # or just one direction: 'forward'|'reverse'|'both' g = g.create_index("edge_out_adj") # or one kind: 'edge_out_adj'|'edge_in_adj'|'node_id' g = g.gfql_index_node_props(["id"]) # secondary indexes on node property columns - g.show_indexes() # pandas DataFrame: kind, engine, n_keys, nbytes, valid + g.show_indexes() # pandas DataFrame: kind, engine, ..., valid, usable, reason g = g.drop_index() # drop all (or drop_index("edge_out_adj")) Unlike ``gfql_index_all()``, an explicit ``create_index("node_id")`` **raises** on @@ -165,6 +165,12 @@ time). Consequences: skipped, never consulted. - ``g.show_indexes()`` reports liveness in the ``valid`` column, so you can see at a glance whether a rebind knocked an index out. +- ``valid`` alone is not "this index will serve your query": indexes are also + **engine-specific**. The ``usable`` column is True only when the index is fresh AND + built for the resolved query engine (shown in ``query_engine``); otherwise ``reason`` + explains the decline — e.g. a polars-built index on a graph whose default queries + resolve to pandas shows ``usable=False`` with an engine-mismatch reason. Pass + ``show_indexes(engine=...)`` to preview an explicit engine choice. - Rebuild by calling ``gfql_index_all()`` again on the rebound ``g``. .. doc-test: skip diff --git a/graphistry/compute/ComputeMixin.py b/graphistry/compute/ComputeMixin.py index 736d5853f8..37a659cac2 100644 --- a/graphistry/compute/ComputeMixin.py +++ b/graphistry/compute/ComputeMixin.py @@ -644,10 +644,16 @@ def drop_index(self, kind: Optional['IndexKind'] = None, *, column: Optional[str from graphistry.compute.gfql.index import drop_index as _di return _di(self, kind, column=column) - def show_indexes(self): - """Return a pandas DataFrame describing resident GFQL indexes (name, kind, column, valid). Empty if none; ``valid=False`` marks a stale index after a frame rebind.""" + def show_indexes(self, engine: EngineAbstractType = 'auto'): + """Return a pandas DataFrame describing resident GFQL indexes (name, kind, column, valid, usable). Empty if none. + + ``valid=False`` marks a stale index after a frame rebind. ``usable`` further + reports whether the index can serve a query under the resolved engine — + indexes are engine-specific, so a fresh index built for another engine + declines to a scan; ``reason`` explains any decline. Pass ``engine=`` to + preview an explicit engine choice (default ``'auto'`` mirrors query resolution).""" from graphistry.compute.gfql.index import show_indexes as _si - return _si(self) + return _si(self, engine=engine) def gfql_index_edges(self, direction='both', engine='auto'): """Convenience: build the edge adjacency index(es) — 'forward', 'reverse', or 'both'. Returns a new Plottable.""" diff --git a/graphistry/compute/gfql/index/api.py b/graphistry/compute/gfql/index/api.py index a58f0f5da4..95bbbfb28e 100644 --- a/graphistry/compute/gfql/index/api.py +++ b/graphistry/compute/gfql/index/api.py @@ -8,7 +8,7 @@ from __future__ import annotations import copy -from typing import Dict, List, Literal, Optional, Sequence, cast +from typing import Dict, List, Literal, Optional, Sequence, Tuple, cast import pandas as pd @@ -81,6 +81,15 @@ def _trace_active() -> bool: return _get_trace_steps() is not None +def _engine_mismatch_text(kinds: str, index_engines: str, engine: Engine) -> str: + """Single wording for an engine-mismatched index decline, shared by the + ``gfql_explain`` trace diagnostic and the ``show_indexes`` ``reason`` column.""" + return ( + f"resident {kinds} index engine={index_engines}, " + f"requested engine={engine.value} -> scan" + ) + + def _engine_mismatch_reason( registry: GfqlIndexRegistry, direction: HopDirection, engine: Engine ) -> Optional[str]: @@ -107,10 +116,25 @@ def _engine_mismatch_reason( return None kinds = ", ".join(kind for kind, _ in mismatches) engines = ", ".join(sorted({idx.engine.value for _, idx in mismatches})) - return ( - f"resident {kinds} index engine={engines}, requested engine={engine.value} " - "-> scan" - ) + return _engine_mismatch_text(kinds, engines, engine) + + +def _index_usability( + kind: IndexKind, index_engine: Engine, valid: bool, query_engine: Engine +) -> Tuple[bool, Optional[str]]: + """Usability of ONE resident index under the resolved query engine. + + ``valid`` (fingerprint freshness) is necessary but not sufficient: indexes are + engine-specific, so a fresh index built for another engine still declines to a + scan at query time (#1767 disposition). Returns ``(usable, reason)`` where + ``reason`` uses the same wording as the ``gfql_explain`` decline diagnostic. + """ + reasons: List[str] = [] + if index_engine != query_engine: + reasons.append(_engine_mismatch_text(kind, index_engine.value, query_engine)) + if not valid: + reasons.append("stale fingerprint (frames rebound since build) -> rebuild") + return (not reasons), ("; ".join(reasons) or None) def _record_indexed_traversal( @@ -308,16 +332,27 @@ def drop_index( return _attach(g, registry.without(kind)) -def show_indexes(g: Plottable) -> pd.DataFrame: +def show_indexes( + g: Plottable, engine: EngineAbstractType = EngineAbstract.AUTO +) -> pd.DataFrame: """Return a pandas DataFrame describing resident indexes (empty if none). ``valid`` reflects live fingerprint validity against the current frames — a stale index (after a ``.edges()``/``.nodes()`` rebind) shows ``valid=False`` and is auto-skipped (scan fallback) until rebuilt. ``nbytes`` is the resident sidecar-array footprint (the pay-as-you-go memory signal). + + ``valid`` alone is NOT "this index will serve your query": indexes are + engine-specific, so a fresh index built for another engine silently declines to + a scan at query time (#1767). ``query_engine`` is what ``engine`` resolves to + for THIS graph (the same resolution a query makes — pass ``engine=`` to preview + an explicit choice), ``usable`` is True only when the index is fresh AND + engine-matched, and ``reason`` says why not, with the same wording as the + ``gfql_explain`` decline diagnostic. """ from .registry import index_nbytes + query_engine = resolve_engine(engine, g) registry = get_registry(g) rows: List[Dict[str, object]] = [] for kind in registry.kinds(): @@ -333,6 +368,7 @@ def show_indexes(g: Plottable) -> pd.DataFrame: valid = g._nodes is not None and registry.get_valid( NODE_ID, g._nodes, (node_idx.key_col,), node_idx.engine) is not None n_keys, n_rows = node_idx.n_nodes, 0 + usable, reason = _index_usability(kind, idx.engine, valid, query_engine) rows.append({ "name": idx.name or index_name(kind, idx.key_col), "kind": kind, @@ -343,9 +379,14 @@ def show_indexes(g: Plottable) -> pd.DataFrame: "n_rows": n_rows, "nbytes": index_nbytes(idx), "valid": valid, + "query_engine": query_engine.value, + "usable": usable, + "reason": reason, }) for column in registry.node_prop_cols(): prop = registry.node_props[column] + prop_valid = registry.get_node_prop_valid(column, g._nodes, prop.engine) is not None + usable, reason = _index_usability(NODE_PROP, prop.engine, prop_valid, query_engine) rows.append({ "name": prop.name or index_name(NODE_PROP, column), "kind": NODE_PROP, @@ -355,9 +396,15 @@ def show_indexes(g: Plottable) -> pd.DataFrame: "n_keys": prop.n_keys, "n_rows": prop.n_nodes, "nbytes": index_nbytes(prop), - "valid": registry.get_node_prop_valid(column, g._nodes, prop.engine) is not None, + "valid": prop_valid, + "query_engine": query_engine.value, + "usable": usable, + "reason": reason, }) - cols = ["name", "kind", "key_col", "engine", "backend", "n_keys", "n_rows", "nbytes", "valid"] + cols = [ + "name", "kind", "key_col", "engine", "backend", "n_keys", "n_rows", "nbytes", + "valid", "query_engine", "usable", "reason", + ] return pd.DataFrame(rows, columns=cols) diff --git a/graphistry/compute/gfql/index/explain.py b/graphistry/compute/gfql/index/explain.py index 1b281c9b26..5c236ab02c 100644 --- a/graphistry/compute/gfql/index/explain.py +++ b/graphistry/compute/gfql/index/explain.py @@ -42,7 +42,7 @@ def gfql_explain( ) -> GfqlExplainReport: resolved_policy: IndexPolicy = validate_index_policy(index_policy) or "use" eng = resolve_engine(engine, g) - resident = show_indexes(g) + resident = show_indexes(g, engine=engine) with index_trace() as steps: try: g.gfql(query, engine=engine, index_policy=resolved_policy) diff --git a/graphistry/compute/gfql/index/wire.py b/graphistry/compute/gfql/index/wire.py index 23ccf2e793..458037933b 100644 --- a/graphistry/compute/gfql/index/wire.py +++ b/graphistry/compute/gfql/index/wire.py @@ -154,5 +154,5 @@ def apply_index_op(g: Any, op: IndexOp, *, engine: Any = "auto") -> Any: raise ValueError(f"DROP GFQL INDEX: no resident index of kind {kind!r}") return drop_index(g, kind, column=op.column) if isinstance(op, ShowIndexes): - return show_indexes(g) + return show_indexes(g, engine=engine) raise ValueError(f"Unknown index op: {op!r}") diff --git a/graphistry/tests/compute/gfql/index/test_index.py b/graphistry/tests/compute/gfql/index/test_index.py index 29116e6208..6a63748fd7 100644 --- a/graphistry/tests/compute/gfql/index/test_index.py +++ b/graphistry/tests/compute/gfql/index/test_index.py @@ -1399,3 +1399,117 @@ def test_engine_mismatch_reason_reports_all_undirected_indexes( "resident edge_out_adj, edge_in_adj index " f"engine={resident_engine}, requested engine={requested_engine} -> scan" ), reason + + +class TestShowIndexesEngineUsability: + """#1767 disposition: ``show_indexes`` must stop reporting an index as fine when + the resolved query engine cannot use it. ``valid`` stays fingerprint-only (BC); + ``query_engine``/``usable``/``reason`` add the engine-usability signal, reusing + #1838's mismatch-reason wording.""" + + def test_matching_engine_reports_usable(self, graph): + g = graph.create_index("edge_out_adj", engine="pandas") + row = g.show_indexes().iloc[0] + assert bool(row["valid"]) is True + assert row["query_engine"] == "pandas" + assert bool(row["usable"]) is True + assert row["reason"] is None + + def test_polars_index_auto_query_not_usable(self, graph): + """The disposition's silent cliff: polars-built index, AUTO query resolves + pandas -> fingerprint-fresh but engine-unusable, with the explain wording.""" + pytest.importorskip("polars") + gi = graph.create_index("edge_out_adj", engine="polars") + row = gi.show_indexes().iloc[0] # AUTO: polars frames resolve to pandas queries + assert bool(row["valid"]) is True # fingerprint is fresh — that was never the bug + assert row["query_engine"] == "pandas" + assert bool(row["usable"]) is False + assert row["reason"] == ( + "resident edge_out_adj index engine=polars, requested engine=pandas -> scan" + ) + + def test_polars_index_explicit_polars_query_usable(self, graph): + pytest.importorskip("polars") + gi = graph.create_index("edge_out_adj", engine="polars") + row = gi.show_indexes(engine="polars").iloc[0] + assert bool(row["valid"]) is True + assert row["query_engine"] == "polars" + assert bool(row["usable"]) is True + assert row["reason"] is None + + def test_pandas_index_explicit_polars_query_not_usable(self, graph): + g = graph.create_index("edge_out_adj", engine="pandas") + row = g.show_indexes(engine="polars").iloc[0] + assert bool(row["valid"]) is True + assert bool(row["usable"]) is False + assert row["reason"] == ( + "resident edge_out_adj index engine=pandas, requested engine=polars -> scan" + ) + + def test_stale_index_not_usable_even_when_engine_matches(self, graph): + g = graph.create_index("edge_out_adj", engine="pandas") + rng = np.random.default_rng(3) + g2 = g.edges( + pd.DataFrame({"src": rng.integers(0, 2000, 50), "dst": rng.integers(0, 2000, 50)}), + "src", "dst", + ) + row = g2.show_indexes().iloc[0] + assert bool(row["valid"]) is False + assert row["query_engine"] == "pandas" + assert bool(row["usable"]) is False + assert row["reason"] == "stale fingerprint (frames rebound since build) -> rebuild" + + def test_stale_and_engine_mismatched_reports_both(self, graph): + pytest.importorskip("polars") + gi = graph.create_index("edge_out_adj", engine="polars") + rng = np.random.default_rng(4) + g2 = gi.edges( + pd.DataFrame({"src": rng.integers(0, 2000, 50), "dst": rng.integers(0, 2000, 50)}), + "src", "dst", + ) + row = g2.show_indexes(engine="polars").iloc[0] + assert bool(row["valid"]) is False + assert bool(row["usable"]) is False + assert row["reason"] == "stale fingerprint (frames rebound since build) -> rebuild" + row_pd = g2.show_indexes(engine="pandas").iloc[0] + assert bool(row_pd["usable"]) is False + assert row_pd["reason"] == ( + "resident edge_out_adj index engine=polars, requested engine=pandas -> scan" + "; stale fingerprint (frames rebound since build) -> rebuild" + ) + + def test_every_kind_carries_per_row_reason(self, graph): + pytest.importorskip("polars") + gi = graph.gfql_index_all(engine="polars") + gi = gi.create_index("node_prop", column="lab", engine="polars") + si = gi.show_indexes() # AUTO -> pandas + assert set(si["kind"]) == {"edge_out_adj", "edge_in_adj", "node_id", "node_prop"} + assert si["valid"].all() + assert not si["usable"].any() + for _, row in si.iterrows(): + assert row["reason"] == ( + f"resident {row['kind']} index engine=polars, " + "requested engine=pandas -> scan" + ) + + def test_show_indexes_ddl_surface_respects_engine(self, graph): + pytest.importorskip("polars") + gi = graph.create_index("edge_out_adj", engine="polars") + si_auto = gi.gfql("SHOW GFQL INDEXES") + assert bool(si_auto.iloc[0]["usable"]) is False + si_pl = gi.gfql("SHOW GFQL INDEXES", engine="polars") + assert bool(si_pl.iloc[0]["usable"]) is True + assert si_pl.iloc[0]["reason"] is None + + def test_cudf_index_pandas_query_not_usable(self, graph): + pytest.importorskip("cudf") + gi = graph.create_index("edge_out_adj", engine="cudf") + row = gi.show_indexes(engine="pandas").iloc[0] + assert bool(row["valid"]) is True + assert bool(row["usable"]) is False + assert row["reason"] == ( + "resident edge_out_adj index engine=cudf, requested engine=pandas -> scan" + ) + row_auto = gi.show_indexes().iloc[0] # AUTO on cudf frames resolves cudf + assert row_auto["query_engine"] == "cudf" + assert bool(row_auto["usable"]) is True