refactor: make xorq an optional (extra) dependency

[deepyaman]

Resolves #270 (and progresses towards #140)

What this does

Moves xorq from a core dependency to an optional [xorq] extra, so pip install boring-semantic-layer works without it. Core querying, YAML round-trips, and named profiles all continue to function against plain ibis backends.

Key changes

_xorq.py — dual-source shim
Wraps all xorq imports in a try/except ImportError block. When xorq is absent, falls back to the equivalent plain ibis-framework symbols. Exposes a HAS_XORQ flag used by call sites to gate xorq-only code paths. Provides pure-Python fallbacks for to_node, walk_nodes, replace_nodes, from_ibis (identity), and a map_ibis stub that preserves the register() interface without doing anything.

profile.py
Gates the XorqProfile lookup on HAS_XORQ; skips xorq_dir discovery when xorq is absent. Extracts _connect_plain_ibis() to avoid duplicating the connection logic across both the xorq-absent path and the existing xorq-AssertionError fallback. Adds _expand_env_vars(), which raises ProfileError on undefined ${VAR} references — os.path.expandvars silently leaves them in place, so without xorq database: ${MISSING} would create a DuckDB file literally named ${MISSING}. This matches xorq's strict behavior and only manifests on the new plain-ibis path.

ops.py / query.py — no-xorq parity
Short-circuits _rebind_to_canonical_backend and SemanticJoinOp backend rebinding when HAS_XORQ is false: without xorq from_ibis() is an identity, so there is nothing to rebind, and returning the inputs unchanged avoids the shim's plain-ibis walk/replace rebuilding an equal-but-distinct expression. Marks dict/string filters with __bsl_deferred_resolution__ so the dimension-only source-table shortcut opts out of static column introspection consistently — without xorq the tracking proxy would otherwise spuriously appear to succeed.

serialization/__init__.py and serialization/tag_handler.py
Guards from_tagged with the same try_import_xorq() + ImportError pattern already used by to_tagged. Sets bsl_tag_handler = None when xorq is absent (the entry point is only loaded by xorq at runtime, so this is safe).

pyproject.toml
xorq>=0.3.25 moves to an [xorq] optional extra; examples now pulls xorq[duckdb]>=0.3.25. The dev extra drops its self-referential boring-semantic-layer[...] bundle and lists the LLM provider clients and developer tooling directly.

CI
The test job is now a matrix over two legs: full (uv sync --all-extras) and no-xorq (--all-extras --no-extra xorq --no-extra examples, ignoring the integration suite). The no-xorq leg verifies xorq is genuinely optional — its tests skip, not error. Examples, docs, and skills (which need Node) move to a dedicated build job. Release gates (deploy-docs, release-pypi) now require both test and build.

Tests
Xorq-only tests standardize on pytest.importorskip("xorq") / a top-level import xorq so the skip fires before any BSL import. Flavor-sensitive tests and the demo adopt the xibis shim idiom for composed expressions. The projection-pushdown xfail is clarified. test_dependency_groups.py is updated for the new extra layout.

[deepyaman]

Another round of feedback.

[deepyaman]

I know I suggested this get_ibis(), but what's the point of Xorq here? The demo is otherwise entirely Ibis-based; why would we introduce a Xorq import just for window?

[deepyaman]

BSL's to_untagged() returns xorq-vendored ibis when xorq is installed, and those expressions reject a plain ibis.window (LegacyWindowBuilder). The demo runs under the [xorq] extra in CI, so plain ibis.window would break there. Switched to the established shim idiom from boring_semantic_layer._xorq import ibis as xibis (as used in expr.py) — plain ibis without xorq, vendored only when present — with an explanatory comment. Also dropped the get_ibis() helper I'd added, since it diverged from that existing idiom. (5d97122)

[deepyaman]

Feel like we should parametrize this workflow, and move the docs stuff into a separate step. Seems cleaner.

[deepyaman]

Done in dd2eb03 — collapsed the two near-duplicate test jobs into a matrix over the xorq extras (full / no-xorq), and split examples + docs-build + skills-check into a dedicated build job. Coverage is preserved; release/deploy gates updated to needs: [test, build].

[deepyaman]

Again, is Xorq necessary, or this could be pure Ibis? Does Xorq.window provide anything? Was there any justification for this in the blame; e.g. if Ibis window was missing some functionality that seems to be fixed now?

[deepyaman]

Checked the blame: the xorq-vendored window came in via 6296e56 / 7328d40 because to_untagged() returns xorq-vendored ibis. It's still required today — this PR's own CI proved it: commit 9a61507 (which used ibis.window) failed the with-xorq job with XorqTypeError: Unexpected window type: LegacyWindowBuilder. So ibis.window is not "fixed" for this case. xorq.window isn't functionally different (same ibis code under xorq's vendored namespace); it's needed only for type identity. Now built via the xibis shim so it's plain ibis without xorq. (5d97122)

[deepyaman]

Should the two SQL generation tests be parametrized?

Also, does doing to_sql specifying dialect actually test what we want? I would expect you need a Trino backend to see the true behavior; not sure if Xorq works differently from Ibis in this regard.

[deepyaman]

Parametrized (5d97122). On the dialect question: ibis.to_sql(expr, dialect="trino") compiles through the dialect's compiler with no live backend — that's the standard, intended way to test dialect SQL generation. The assertion is on the emitted SQL string (a compilation concern), not execution, so a Trino backend isn't needed; xorq's vendored to_sql uses the same compiler. (This test predates the PR; we only swapped the ibis flavor.)

[deepyaman]

Comment necessary?

If it is, I'd expect it for the below test, too. The need for commenting twice is also why I suggested parametrizing the test above.

[deepyaman]

Parametrized the two conversion tests into one (5d97122); the single result.type().is_timestamp() assertion is self-explanatory, so the comment is gone.

[deepyaman]

Is this true? Maybe should investigate/be very sure of this.

If this was the reason for making Xorq a core dependency, then we have a clear justification for making it optional again, if we can confidently prove whatever window issue is resolved. Please consider this carefully.

[deepyaman]

Investigated carefully. The window-compat reason was real, but rooted in BSL vendoring (to_untagged() returns xorq-vendored ibis, 7328d40), not a missing ibis feature — and it is not "resolved": plain ibis.window still fails under xorq (verified by this PR's CI, commit 9a61507). It doesn't need to be resolved to make xorq optional, though — the incompatibility only exists when xorq is installed, and the shim selects the right flavor per environment. The dep-group note now describes the extra accurately (serialization/caching; optional).

[deepyaman]

Is importorskip not more idiomatic? Is there a reason for doing it this way?

[deepyaman]

Agreed — converted the three remaining files (test_malloy_xorq_roundtrip, test_xorq_integration, test_xorq_backends) to module-level pytest.importorskip("xorq"), matching the 11 files that already use it. Left test_dependency_groups.py as-is since its purpose is to test the dependency-group mechanism itself. (05e7b4a)

[deepyaman]

This seems like a separate fix that should, at the very least, be a separated commit. Probably, it could be its own PR, right?

[deepyaman]

Nvm, maybe the connect plain Ibis and stuff indicate that this is coupled to this PR.

[deepyaman]

Split into its own commit: 1d64bdb.

[deepyaman]

Right — it's coupled: the bug only manifests on the new no-xorq _connect_plain_ibis() path (xorq's path handled env-var expansion). Kept in this PR, but as its own commit (1d64bdb).

[deepyaman]

In addition to the below comments, want to clarify that should only update Ibis/Xorq capitalization in this PR if it's justifiably consistent with what exists across the codebase. Don't want to just capitalize whatever was introduced in this PR, if it's not in line with the rest of the codebase.

[deepyaman]

Is pytest_args the way to go for this? Should it not be markers based on whether Xorq is installed or something?

[deepyaman]

Good instinct — markers/conditional skips are the right model, and the xorq-specific unit tests already do exactly that via importorskip/HAS_XORQ guards. This --ignore was narrower: it targets tests/integration/, whose BSL query modules (malloy_data/*.py) import xorq.api directly, so the whole directory genuinely requires xorq.

I moved that gate into the test tree — integration/conftest.py now sets collect_ignore_glob = ["*"] when import xorq fails, so the directory self-skips without xorq. Both CI legs now run a plain uv run pytest; pytest_args is gone. (8d1fbc6)

[deepyaman]

Why is this necessary? Was something like this there before?

[deepyaman]

Yes — it predates this refactor: main's test-no-xorq job had a Verify import works without xorq step. It's a cheap smoke test that separates "the package does not import" from "a test failed" — especially meaningful for the no-xorq leg, whose whole purpose is proving BSL imports cleanly without xorq before the slower suite runs. The matrix just kept it. Happy to gate it to no-xorq only if you'd rather not have the (redundant) run on the full leg.

[deepyaman]

Nit: Xorq-vendored Ibis? Should it not be hyphenated?

Also, in general I'd capitalize Ibis personally... I assume Xorq should be a proper name, the same. At the same time, there's also the consistency with existing comments and stuff, regardless of grammatical correctness, so use your judgement I guess.

[deepyaman]

Hyphenated to xorq-vendored ibis — that's already the dominant form in the codebase (ops.py alone uses "xorq-vendored" 6×, plus nested_compile.py, mcp.py, server/api.py, …). Kept both lowercase deliberately: the codebase consistently lowercases xorq/ibis mid-sentence (package names) and only capitalizes at sentence start, so capitalizing here would break consistency rather than improve it. Applied the same fix to the two sibling pushdown test files that share the string. (b649f34)

[deepyaman]

If this is not reachable, then why is it necessary? Is there a good justification?

[deepyaman]

It keeps the shim's symbol surface symmetric: every name the xorq branch exports is importable from the no-xorq branch too, so the function-local from ._xorq import ..., walk_nodes at the call sites in ops.py/reconstruct.py still resolves. You're right the body is unreachable without xorq — those callers all bail on HAS_XORQ first — so the raise is defense-in-depth: a clear, actionable error if some future un-gated caller reaches it, instead of a cryptic cannot import name. Reworded the comment to say that rather than just "should never run." (b649f34)