Harden CLI validation and release workflows#183
Conversation
nicosuave
force-pushed
the
review/cli-ci-release-hardening
branch
from
326838a to
c6b38ca
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6b38ca756
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if parent is None: | ||
| return None | ||
|
|
||
| merged = merge_model(model, parent) |
There was a problem hiding this comment.
When loading native models with extends through the normal CLI/API path, the target SemanticLayer is still the current auto-registration layer here. merge_model() reconstructs a new Model and nested Metric objects, whose constructors auto-register with the current layer; for inherited model-level metrics with no agg or with type: derived/ratio, that leaks them into layer.graph.metrics before the final registration pass. This only shows up for native inheritance, and can make model-scoped inherited metrics appear as graph-level metrics or collide with other graph metrics; suppress the current layer around the merge just like adapter parsing does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c25d80cef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| def load_from_directory(layer: "SemanticLayer", directory: str | Path) -> None: | ||
| def load_from_directory(layer: "SemanticLayer", directory: str | Path, *, strict: bool = True) -> None: |
There was a problem hiding this comment.
With strict=True now documented as failing on parse errors, Graphene .gsql projects still bypass that behavior because load_from_directory() calls _load_graphene_project() before the per-file adapter loop, and that helper continues to catch adapter exceptions, log a warning, and return. In a directory that contains a malformed Graphene model plus any other valid model file, sidemantic validate/info can still pass while silently dropping the Graphene project, unlike the other detected semantic formats hardened in this change.
Useful? React with 👍 / 👎.
1 participant
Makes base
sidemantic validatenoninteractive and independent of the workbench extra, adds missing-extra hints forserveand charts, updates CLI-first docs/examples, restores the Rust parity matrix, documents the Pyodide no-deps path, and hardens release/DuckDB dependency workflow setup.Validation:
uv run ruff check . --exclude docs/_extensions --exclude sidemantic-duckdb/extension-ci-tools --exclude sidemantic-duckdb/scripts --exclude sidemantic-duckdb/duckdb --exclude sidemantic/adapters/malloy_grammar --exclude sidemantic/adapters/holistics_grammaruv run ruff format --check . --exclude docs/_extensions --exclude sidemantic-duckdb/extension-ci-tools --exclude sidemantic-duckdb/scripts --exclude sidemantic-duckdb/duckdb --exclude sidemantic/adapters/malloy_grammar --exclude sidemantic/adapters/holistics_grammaruv run pytest -v