Skip to content

Commit a56f998

Browse files
lmeyerovclaude
andcommitted
docs(changelog): the two entries missing from #1792 and #1793
Both landed without a CHANGELOG entry. #1793 is the one that matters: it fixes a SILENT WRONG ANSWER that is user-visible on every engine. Reproduced on the pre-fix tree while auditing the merge, so the entry states a measured effect rather than a description: `MATCH (a {grp:1}) WITH a MATCH (a)-[]->(b) RETURN count(*)` followed by a plain `MATCH (a)-[]->(b) RETURN count(*)` on the SAME graph object returned 60 instead of 134, 69 instead of 136 and 61 instead of 143 across seeds. On polars the poisoned graph then raised `NotImplementedError` for EVERY subsequent query — a failed query left the user's object broken. Both are fixed on master; neither was written down. #1792's graphviz half (`KeyError: None` -> an actionable `ValueError` on a bound-but-unlabelled frame) is smaller but is still a user-facing error-message change, so it gets an entry too. DOCS ONLY: no code touched, so runtime delta is zero and no pyg-bench lane run is required (CB5) — checkable from the diff, which is one file. NOT INCLUDED, deliberately. The audit also flagged #1792's `dtype: object` -> `dtype: DType` as a typing WIDENING (`DType = Any`, which is strictly weaker than `object`, and mypy is indifferent between them here — verified). I am not reverting it: `graphistry/compute/typing.py` declares `DType` as the repo's engine-agnostic dtype alias with the comment "Honestly Any -- the concrete type is engine-dependent", and these parameters do receive pandas/cuDF/polars dtypes, so the named alias is the documented convention even though it checks as `Any`. Reverting on the audit's reading would churn against a stated convention on a judgement call that belongs to the owner. Reported, not silently decided. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
1 parent 233b64c commit a56f998

1 file changed

Lines changed: 2 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2828
- **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.
2929

3030
### Fixed
31+
- **A query no longer mutates the `Plottable` it was run on (#1786)**: `WITH` re-entry seeds the follow-up `MATCH` from the carried nodes, and that seed was assigned to the CALLER's graph and never cleared. So the next — entirely unrelated — query on the same object was answered against the stale seed: no error, just the previous query's count. Reproduced on the pre-fix tree: `MATCH (a {grp:1}) WITH a MATCH (a)-[]->(b) RETURN count(*)` followed by a plain `MATCH (a)-[]->(b) RETURN count(*)` on the SAME graph returned 60 instead of 134, 69 instead of 136, 61 instead of 143 across seeds; on polars the poisoned graph then raised `NotImplementedError` for every subsequent query, so a failed query left the user's object broken. Engine-independent (wrong on pandas too), which is why an engine A/B never surfaced it, and a direct violation of the library's pure-functional contract — users reasonably reuse one `Plottable` for many queries. Every `_gfql_*` field assigned during execution was audited: the reported `gfql_unified` seed write (where `_seeded_dispatch_graph` hands back the caller's `base_graph` itself when there are no seed rows), the per-CALL `shortest_path_backend` argument that persisted on the caller's graph and silently became the default for its next query, and the two chain boundary handlers. All now carry the state on an INTERNAL copy and clear it on the way out — the second half matters independently, because the RESULT of a `WITH` query used to carry the seed, so a follow-up query on that result (a different graph entirely) got the same wrong answer one hop removed. **This entry was missing when the fix landed** and is added here rather than left out; it is the most user-visible of the changes in this release.
32+
- **`layout_graphviz` / `render_graphviz` raised a bare `KeyError: None` on a bound-but-unlabelled frame**: `g_to_pgv` asserted that `_nodes`/`_edges` are set but not that the BINDINGS are, and `g.nodes(df)` with no `node=` (or `g.edges(df)` with no source/destination) leaves them `None`. Those graphs reached `row[None]` and died inside the row loop with an error naming nothing the caller could act on. The bindings are now resolved once, up front, into non-Optional locals with an actionable `ValueError`, and the validation runs BEFORE the optional `pygraphviz` import so a caller error is not masked by a missing-backend error. **This entry was also missing when the fix landed.**
3133
- **`rows(table='edges')` silently returned the wrong table after a NAMED traversal**: the named-middle rewrite — which turns `[...named ops..., rows()]` into `rows(binding_ops=...)` so a Cypher multi-alias `RETURN` lowers to a bindings table — skipped itself when the call already carried `binding_ops`, `source` or `alias_endpoints`, but not when it carried a non-default `table`. So naming any op in the middle changed which table came back: an odd-length middle silently produced the BINDINGS table instead of the requested edges (no error, `table=` simply ignored), and an even-length middle — a path ending on an edge, e.g. `(person)-[r:KNOWS]-` — produced a non-alternating op list and raised "require ... a single connected alternating node/edge path". An unnamed middle was unaffected, which is what made it look shape-specific rather than naming-specific. Both chain surfaces are fixed (the generic chain and the native polars chain carry the rewrite independently, so fixing one left the other wrong). Note the guard keys on a NON-DEFAULT table: `rows()` declares `table='nodes'` and always emits it, so an explicit `rows(table='nodes')` is indistinguishable from a bare `rows()` and still rewrites — pinned as a known limitation rather than left to be rediscovered.
3234
- **Indexed bindings bypass returned the WHOLE edge table for `rows(table='edges')`**: the bypass exists to SKIP the canonical traversal, so whatever it hands the suffix is the pre-traversal graph. That is sound for a bindings table — the indexed path bag already is the answer — but the gate declined only for `source` / `alias_endpoints` / `alias_prefilters`, not for a non-default `table`, so a `rows(table='edges')` after a named middle read the full edge frame instead of the traversal-narrowed one (measured on a 12-edge fixture: 12 rows instead of 3, on pandas, cuDF and native polars, and the wrong values survive a following `select`). Both gates now decline a non-default `table` (`chain._plan_indexed_middle` and the native polars `_try_indexed_middle_polars`), matching the named-middle rewrite's guard. This became reachable only once that rewrite stopped firing for a non-default `table` — before it, the rewrite converted the call on BOTH the indexed and the scan path, so the two agreed on the wrong table. Same class as the earlier "indexed bypass could return every node for `rows()` over an unnamed pattern".
3335
- **The named-middle rewrite discarded the caller's other `rows()` params**: both rewrites built a FRESH `rows(binding_ops=...)` instead of adding `binding_ops` to the call the caller wrote, so every param the rewrite has no opinion about was thrown away. For `attach_prop_aliases` — the projection pushdown — that is observable: spelling a pattern as a NAMED middle rather than as explicit `binding_ops` silently attached every alias's properties instead of the requested ones, i.e. the same query returned a wider schema depending only on how it was written. `alias_prefilters` was dropped the same way. Both are now carried through, on both chain surfaces. Known remaining gap, pinned by a strict-xfail test rather than left to be rediscovered: the native polars bindings builder never receives `alias_prefilters` at all, so hand-written GFQL passing that hint without an equivalent post-filter still gets engine-dependent row counts (Cypher-generated plans always keep the post-filter, so they agree across engines and only lose the pushdown).

0 commit comments

Comments
 (0)