diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f844842c8..26db470125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) **zero LALR conflicts** (dependency-free, always-on in CI) plus a `strict=True` build test (skipped where the optional `interegular` dep is absent) — a grammar edit that introduces any ambiguity fails CI; (2) semantic ambiguity is ZERO — every corpus query has exactly one Earley derivation and it transforms to a single AST, no exceptions; (3) a rule-coverage gate (`test_every_grammar_rule_is_exercised_by_the_corpus`) forces every new grammar rule through the invariants; (4) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and a small set of deliberate language fixes — accept-by-accident shapes with ill-defined semantics, now honest syntax errors and pinned as tests (`DELIBERATE_LANGUAGE_FIXES`): `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, WHERE before MATCH in a graph constructor, and the invalid "list of IN-booleans" (`[x IN xs, y]`; use `[(x IN xs), y]` for a genuine bool list). The lift stays an internal optimization: only a flat `AND` chain over present columns lifts to `filter_dict`; parens / OR / XOR / NOT and any absent-column case stay on `where_rows` (a correctness boundary, since `where_rows` treats an absent property as null while `filter_dict` would raise). Full cypher suite (1,681 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. A pre-existing `<>`-over-null 3VL divergence between the two execution paths (surfaced by #1653's metamorphic test) is tracked separately in #1683. ### Fixed +- **`_switch_org` no longer treats a rejected organization switch as a success**: `ArrowUploader._switch_org` posted to `/api/v2/o//switch/`, passed the response to `log_requests_error` (which only logs, never raises), and then cached the `(org, token)` pair unconditionally. A server that rejected the switch was therefore recorded as having accepted it, and the early return at the top of the method turned every later attempt into a silent no-op for the rest of the session. The method now records the pair only on a 2xx response, returns a bool so callers can tell acceptance from rejection, and logs a warning naming the status code. A rejected switch is retried on the next call rather than skipped. Login still succeeds when the switch fails, since not every deployment exposes the endpoint. +- **Login no longer crashes when the server sends a null `active_organization`**: `_finalize_login` read `json_response.get('active_organization', {})` and then called `.get('slug')` on the result. The dict default only covers a missing key, so a server sending `"active_organization": null` (or any non-dict) raised `AttributeError` instead of being read as "no organization bound". The username/password and personal-key login paths now apply the same `isinstance` guard the SSO path already used. - **GFQL Cypher `GRAPH { }` residual predicates now fail safely or apply as graph masks**: `GRAPH { MATCH ... WHERE ... }` no longer silently drops predicates that the graph-state path cannot apply. Safe one-node/one-edge residual filters, including disjunctions and `searchAny(...)`, are applied before graph-state matching; unsupported pattern-predicate, multi-alias, and Polars graph-residual cases now raise clear validation errors instead of returning an over-broad subgraph. - **GFQL Cypher `=~` / scalar-fn cross-engine hardening (review wave, dgx-verified)**: (1) composed `=~` (`WHERE … =~ … OR …`, `RETURN`-expression position) now works on `engine='cudf'` — the series evaluator used raw `.str.fullmatch`, which cuDF lacks, and the resulting `AttributeError` was masked as "unsupported predicate op" (it now routes through the `Fullmatch` predicate's engine workarounds, and honest `NotImplementedError` declines pass through instead of being re-labeled); (2) the cuDF fullmatch emulation anchors alternations as a whole (`^(ab|cd)$` — bare `^ab|cd$` silently matched `'abXXX'`); (3) the cuDF case-insensitive `(?i)` lowercase-folding workaround now declines the fold-unsound shapes — uppercase escape classes (`.lower()` turns `\D` into `\d`, silently inverting the predicate), case-crossing character ranges (`(?i)[A-z]` silently narrowed; `[X-b]` folded to an invalid range), and non-ASCII patterns — while lowercase escapes (`\d`, `\.`) keep folding as before; lookaround, backreferences, and named-group refs decline up front (libcudf rejects them at kernel-compile time); (4) polars `Match`/`Fullmatch` lowering applies the same Rust-regex guard as `Contains` (lookaround/backrefs decline instead of a non-NIE `ComputeError` at collect); (5) `toLower`/`toUpper` on a non-string column decline like neo4j's type error instead of broadcasting the stringified Series repr (pandas/cuDF) or raising a non-NIE `SchemaError` (polars-gpu); (6) polars `floor`/`ceil`/`round` cast to `Float64` so integer columns return Float like neo4j and the pandas engine; (7) invalid regex patterns on the composed path raise a clear "invalid regex pattern" error instead of "unsupported predicate op". - **GFQL Cypher `round()` hardening (review wave, dgx-verified)**: (1) the `polars` extra's floor is now `polars>=1.29` — `Expr.round(mode=)` shipped in py-1.29.0 (pola-rs/polars#22248), not 1.5 as previously pinned, so 1.5–1.28 installs crashed with a raw `TypeError` on `round(x, p>0)` under `engine='polars'`; (2) `round(x, p>308)` is the identity on both engines (a float64 has no digits there) instead of pandas raising through an unclear decline while polars returned identity — parity restored, `10.0**p` overflow guarded; (3) polars `round(x, p>0)` normalizes `-0.0` to `+0.0` like the pandas kernel (`round(-0.04, 1)` was `0.0` on pandas vs `-0.0` on polars — invisible to value equality, pinned by a sign-bit test); (4) documented the precision>0 decimal-string deviation vs neo4j (`round(2.675, 2)` = `2.67` binary-double here vs `2.68` BigDecimal there) and added deterministic tie/hazard matrix cases so ties actually reach cuDF/polars-gpu (the fixture's normal-distribution floats never tied). diff --git a/graphistry/arrow_uploader.py b/graphistry/arrow_uploader.py index 91cb6b5f63..e08b9ac1e4 100644 --- a/graphistry/arrow_uploader.py +++ b/graphistry/arrow_uploader.py @@ -266,27 +266,46 @@ def certificate_validation(self, certificate_validation): ########################################################################3 - def _switch_org(self, org_name: Optional[str], token: Optional[str]) -> None: + def _switch_org(self, org_name: Optional[str], token: Optional[str]) -> bool: + """ + Ask the server to make org_name the session's active organization. + + Returns True when the server accepted the switch, False otherwise. A rejected + or failed switch is not recorded, so a later call retries instead of silently + skipping. Callers must not treat False as fatal: not every deployment exposes + the switch endpoint, and login should still succeed. + """ if not org_name or not token: - return + return False last = self._client_session._last_switched_org_token if last == (org_name, token): - return + return True + + switch_url = f"{self.server_base_path}/api/v2/o/{org_name}/switch/" try: - switch_url = f"{self.server_base_path}/api/v2/o/{org_name}/switch/" response = requests.post( switch_url, data={'slug': org_name}, headers=inject_trace_headers({'Authorization': f'Bearer {token}'}), verify=self.certificate_validation, ) - log_requests_error(response) - self._client_session._last_switched_org_token = (org_name, token) - from .pygraphistry import PyGraphistry - if PyGraphistry.session is self._client_session: - PyGraphistry.session._last_switched_org_token = (org_name, token) except Exception as exc: logger.warning("Failed to switch organization %s: %s", org_name, exc) + return False + + log_requests_error(response) + if not (200 <= response.status_code < 300): + logger.warning( + "Server rejected switch to organization %s with HTTP %s; " + "the session may remain bound to a different organization", + org_name, response.status_code) + return False + + self._client_session._last_switched_org_token = (org_name, token) + from .pygraphistry import PyGraphistry + if PyGraphistry.session is self._client_session: + PyGraphistry.session._last_switched_org_token = (org_name, token) + return True def login(self, username, password, org_name=None): @@ -338,7 +357,12 @@ def _finalize_login(self, out: requests.Response, org_name: Optional[str]) -> 'A f"missing 'token' key in response. Response content: {out.text}" ) - org = json_response.get('active_organization',{}) + # A server may omit active_organization, send null, or send a non-dict. + # All three mean "no organization bound". The dict default alone does not + # cover a present-but-null key, which would raise AttributeError below. + org = json_response.get('active_organization', None) + if not isinstance(org, dict): + org = {} logged_in_org_name = org.get('slug', None) if org_name: # caller pass in org_name if not logged_in_org_name: # no active_organization in JWT payload diff --git a/graphistry/tests/test_arrow_uploader.py b/graphistry/tests/test_arrow_uploader.py index 867ea66024..f34ab4274d 100644 --- a/graphistry/tests/test_arrow_uploader.py +++ b/graphistry/tests/test_arrow_uploader.py @@ -809,3 +809,116 @@ def test_payload_length_multiple_of_4_after_rstrip_decodes_correctly(self): token = _make_test_jwt(payload_dict) # The exact byte length will vary; the corrected formula is robust to all. assert _personal_org_from_jwt(token) == 'u' + + +class TestArrowUploader_SwitchOrg(unittest.TestCase): + """ + _switch_org must report whether the server accepted the switch, and must not + remember a switch that never happened. Earlier revisions logged the failure and + then cached the (org, token) pair anyway, so the early return turned every later + attempt into a silent no-op. + """ + + def _mock_response(self, status=200, json_data=None): + mock_resp = mock.Mock() + mock_resp.status_code = status + mock_resp.content = "CONTENT" + mock_resp.raise_for_status = mock.Mock() + mock_resp.json = mock.Mock(return_value=json_data if json_data else {}) + return mock_resp + + def _uploader(self): + au = ArrowUploader() + au._client_session._last_switched_org_token = None + return au + + @mock.patch('requests.post') + def test_switch_org_accepted_is_reported_and_cached(self, mock_post): + mock_post.return_value = self._mock_response(status=200) + + au = self._uploader() + assert au._switch_org("my-org", "tok") is True + assert au._client_session._last_switched_org_token == ("my-org", "tok") + + @mock.patch('requests.post') + def test_switch_org_rejected_is_reported_and_not_cached(self, mock_post): + mock_post.return_value = self._mock_response(status=403) + + au = self._uploader() + assert au._switch_org("my-org", "tok") is False + assert au._client_session._last_switched_org_token is None + + @mock.patch('requests.post') + def test_switch_org_rejection_does_not_suppress_a_retry(self, mock_post): + mock_post.return_value = self._mock_response(status=403) + au = self._uploader() + assert au._switch_org("my-org", "tok") is False + + mock_post.return_value = self._mock_response(status=200) + assert au._switch_org("my-org", "tok") is True + assert mock_post.call_count == 2 + + @mock.patch('requests.post') + def test_switch_org_transport_failure_is_reported_and_not_cached(self, mock_post): + mock_post.side_effect = Exception("connection reset") + + au = self._uploader() + assert au._switch_org("my-org", "tok") is False + assert au._client_session._last_switched_org_token is None + + @mock.patch('requests.post') + def test_switch_org_skips_http_when_already_switched(self, mock_post): + au = self._uploader() + au._client_session._last_switched_org_token = ("my-org", "tok") + + assert au._switch_org("my-org", "tok") is True + mock_post.assert_not_called() + + @mock.patch('requests.post') + def test_switch_org_without_org_or_token_is_a_noop(self, mock_post): + au = self._uploader() + assert au._switch_org(None, "tok") is False + assert au._switch_org("my-org", None) is False + mock_post.assert_not_called() + + +class TestArrowUploader_NullActiveOrganization(unittest.TestCase): + """ + A server may omit active_organization, send null, or send a non-dict. The dict + default on .get() only covers a missing key, so a present-but-null value used to + raise AttributeError on the login path. + """ + + def _mock_response(self, json_data): + mock_resp = mock.Mock() + mock_resp.status_code = 200 + mock_resp.content = "CONTENT" + mock_resp.raise_for_status = mock.Mock() + mock_resp.json = mock.Mock(return_value=json_data) + return mock_resp + + @mock.patch('requests.post') + def test_login_with_null_active_organization(self, mock_post): + mock_post.return_value = self._mock_response( + {'token': '123', 'active_organization': None}) + + au = ArrowUploader() + assert au.login(username="u", password="p").token == "123" + assert PyGraphistry.org_name() is None + + @mock.patch('requests.post') + def test_login_with_missing_active_organization(self, mock_post): + mock_post.return_value = self._mock_response({'token': '123'}) + + au = ArrowUploader() + assert au.login(username="u", password="p").token == "123" + assert PyGraphistry.org_name() is None + + @mock.patch('requests.post') + def test_login_with_non_dict_active_organization(self, mock_post): + mock_post.return_value = self._mock_response( + {'token': '123', 'active_organization': 'unexpected-string-shape'}) + + au = ArrowUploader() + assert au.login(username="u", password="p").token == "123" + assert PyGraphistry.org_name() is None