fix: 0.7.13 batched fixes (HTTP backend integrity, query platform filter, trace depth, SQL lineage extraction) by vigneshnarayanaswamy · Pull Request #36 · block/model-ledger

vigneshnarayanaswamy · 2026-07-20T22:37:03Z

Summary

Batched fix release. The headline items are all silent-wrong-result bugs:

Using the SDK against a remote server (Ledger(HttpLedgerBackend(url)), the path the backends guide advertises) corrupted the remote ledger: every recorded event was attached to an auto-registered phantom model named "", registration double-fired, and get_snapshot() always returned None. That whole path now round-trips correctly, and the server rejects empty or whitespace-only model names (422) so older clients fail loudly instead of corrupting the inventory.
The query tool's platform filter was advertised in the schema but never applied: any platform value returned the full inventory. It now filters, and a schema-vs-implementation parity test guards every QueryInput field.
trace fabricated per-node depth from flat-list position, so a model with N transitive dependencies always rendered as an N-deep linear chain. Depth is now the real BFS level, and depth bounds the traversal itself instead of filtering after a full walk.
SQL lineage extraction silently dropped unqualified table names (FROM raw_events parsed to zero tables) and counted CTE names and DELETE FROM targets as read tables, producing missing or phantom lineage on common SQL.

Version is bumped to 0.7.13 and the CHANGELOG is backfilled for 0.7.10 through 0.7.12, which had no entries.

Fixes

HTTP backend integrity

fdabaf5 fix(http): append_snapshot sends the real model name instead of ""; registration logs exactly one registered event (re-registration is idempotent); get_snapshot() is implemented (cache plus changelog reconstruction) instead of always None; empty model_name rejected server-side.
2b53755 fix(http): list_snapshots fetches the model's full history instead of inheriting the changelog's 7-day default, which made get_snapshot/latest_snapshot/Ledger.tag and the platform filter fail on any model older than a week; list_models also caches its name mappings so the hashes it hands out stay resolvable.
a757f29 fix(http): registration payload, tier, and actor reach the server (new optional backend register_model hook; previously payload was dropped and every remote registration landed as tier="unclassified", actor="user").

Query

0c21564 fix: the query tool applies the platform filter instead of silently ignoring it; parity test asserts every QueryInput filter field is consumed.

Trace

a042677 fix(trace): depth is the real BFS level (shortest edge distance, one entry per node at its minimum depth), not flat-list position; input.depth terminates the traversal, so bounded queries do bounded work.
dd6039b fix(tools): total_nodes counts distinct models with direction="both" (nodes reachable both ways were double-counted); whitespace-only model names rejected at validation; new public read-only Ledger.backend property replaces tool-internal _backend access.

SQL lineage extraction

3fd3f7c fix: bare (unqualified) table names are extracted instead of silently dropped.
02fbe6b fix(sql): CTE names (including nested and RECURSIVE forms) and DELETE FROM targets are excluded from read tables; INSERT OVERWRITE TABLE t is recognized.
5349142 fix: strip_template_vars strips spaced template variables like {{ ds }}; docstring example corrected to the actual output.

Introspection dispatch

3150653 fix: XGBoost sklearn-API models dispatch to the xgboost introspector; SklearnIntrospector.can_handle rejects estimators from wrapper frameworks with dedicated introspectors, making dispatch independent of entry-point ordering.
9300c91 test(introspect): dispatch tests execute in every environment via stub framework modules (previously they skipped wherever the ML libraries were absent).

Connectors

166ae70 fix: sql_connector raises a clear TypeError naming the dict-row requirement on tuple-row connections, instead of an opaque indexing error; connectors guide documents the requirement.
9873c10 fix(connectors): rows with keys() but no .get() (e.g. sqlite3.Row) are accepted, not rejected.

Construction guards

a457d66 fix: create_app()/create_server() validate the backend at construction with a clear TypeError (with hints for the common Ledger-instead-of-backend and path-string mistakes) instead of an AttributeError deep inside the first request.
decf659 fix(backends): that validation is a duck check on the five core methods, not full-protocol isinstance, so partial custom backends that worked on 0.7.12 keep working.

CLI, packaging, docs, tests

cc55c41 fix(cli): export --output help says "Output file path" (it writes a single file, not a directory) and the success message prints the real artifact path.
8dfb42c + 086a604 chore: the unused excel extra is deprecated rather than removed, and the export default filename is unchanged; both removals are deferred to 0.8.0 to keep this release patch-safe.
f471de8 test: pandas-dependent tests skip instead of hard-failing when pandas is absent alongside the ML libs.
a0e096c test: use generic identifiers in the DataPort case-normalization test.
efa2fd5 docs: CHANGELOG backfilled for 0.7.10-0.7.12, 0.7.13 entry added, version bumped.
b2e058a docs: snapshot-identity claims corrected (changelog-rebuilt hashes match server-minted identities; client-created hashes are process-local, cache-resolvable only); CHANGELOG updated for the second fix round.

Behavior notes

Trace depth semantics changed on purpose. Reported depth values were fabricated before; on any fan-out graph they now reflect real BFS levels, and node ordering is BFS order rather than changelog-derived order. Linear chains are unaffected. Documented in the CHANGELOG.
Re-registration is idempotent. Registering an existing model returns is_new_model=False and appends no duplicate registered event (previously each repeat appended one). Changing owner/metadata is an update, not a registration.
Empty and whitespace-only model names are rejected (422 over REST, ValidationError at construction). Previously an empty name silently corrupted the inventory.
Deprecations for 0.8.0, noted in the CHANGELOG: the excel extra (unused since the scanner removal) and strict full-protocol backend validation.

Testing

Full suite at this HEAD in a CI-equivalent environment: 840 passed, 29 skipped, coverage 76.7% (floor 70). ruff check, ruff format --check, and mypy src/ clean.
Also green in environments with the ML introspection libs installed, both without pandas (854 passed, 24 skipped; this configuration had 6 hard failures on 0.7.12) and with pandas (864 passed, 21 skipped).
Every fix followed fail-before/pass-after: the new tests were run against the pre-fix tree and confirmed to fail for the advertised reason before the fix landed (e.g. the new HTTP integration suite fails 8 of 9 on 0.7.12; the trace depth tests fail 6; the platform-filter tests fail 4).
The HTTP path was additionally smoke-tested over a real uvicorn server (not TestClient): SDK register/record/list/get_snapshot round-trip with no phantom model and exactly one registered event, plus platform-filter probes against the live server and a persisted SQLite ledger, including 30-day-aged events to exercise the history-window fix.
The new integration suite (tests/test_backends/test_http_ledger_integration.py) drives the SDK against the real create_app() handlers rather than mocks, so the corruption class fixed here can no longer pass unnoticed.

Compatibility

Patch release: no public surface is removed or changed incompatibly; new parameters (Ledger.register(payload=...), RecordInput.tier, the register_model backend hook) are additive, and the two intentional behavior changes (trace depth, registration idempotency) replace outputs that were wrong.
Validation tightening only rejects inputs that previously corrupted data; the excel extra and strict backend validation removals are deferred to 0.8.0.

Ledger(HttpLedgerBackend(url)) — the documented remote-backend path — corrupted the remote ledger three ways: - append_snapshot POSTed /record with model_name: "" (a comment claimed it was resolved server-side; it was not), so the server auto-registered a model literally named "" and attached every recorded event to it. It now resolves the real model name from the hash-to-name cache and raises ModelNotFoundError for unresolvable hashes instead of corrupting the remote inventory. Responses are raise_for_status()-checked. - register() double-logged the registration: save_model's POST /record already logs the registered event server-side, and the SDK's follow-up append_snapshot(registered) posted it again. The backend now skips that redundant post, and the record tool itself logs exactly one registered event per new model (Ledger.register grew an optional payload= merge so the caller's registration payload still lands on the single event). Re-registering an existing model is now idempotent (is_new_model=False, pointing at the original registration event) instead of appending duplicate registered events. - get_snapshot() always returned None. It now serves appended snapshots from a client-side cache and reconstructs older ones from GET /changelog (snapshot hashes are content-derived, so rebuilt snapshots recompute the same identity the server holds). Defense in depth: RecordInput.model_name now requires min_length=1, so old clients that still POST an empty model_name get a 422 instead of silently corrupting the ledger. New integration suite drives Ledger over HttpLedgerBackend against the real create_app() handlers (TestClient, not mocks): no phantom model, exactly one registered event, events attach to the named model, get_snapshot round-trips, and empty model_name is rejected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

QueryInput, the REST /query endpoint, and the MCP query tool all advertise a platform parameter, but query() never read input.platform — any platform value returned the full inventory. Silently-wrong results for a documented filter. Platform is derived from snapshot data (newest discovered payload, then snapshot source), not a column on the model row, so it cannot ride the structural list_models pushdown. query() now resolves platforms for all structurally-matched models in one batched dispatch (batch_platforms — a single SQL round trip on SQL-backed backends) and filters/paginates on the result. An unknown platform now returns 0 models, not 35k. Adds platform filter tests (match, zero-match, text combination, pagination) and a schema-vs-implementation parity test that fails if QueryInput ever grows a filter field query() does not consume. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…trospector Entry points load sorted by name (lightgbm < sklearn < xgboost) and find() returns the first can_handle match. XGBClassifier & friends are isinstance of sklearn.BaseEstimator, so the generic sklearn introspector claimed them and xgboost-specific extraction was lost (result carried introspector='sklearn', framework='scikit-learn'). LightGBM only escaped because 'lightgbm' happens to sort before 'sklearn'. SklearnIntrospector.can_handle now rejects estimators whose class comes from a wrapper framework with a dedicated introspector (xgboost.*, lightgbm.* modules), making dispatch independent of registry order. New dispatch tests (importorskip'd on the ML libs): XGBClassifier -> xgboost, LGBMClassifier -> lightgbm, plain LogisticRegression -> sklearn, and an order-independence check on can_handle itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

extract_tables_from_sql / extract_write_tables required 1-2 dotted segments, so INSERT INTO feature_store SELECT * FROM raw_events parsed to zero reads and zero writes — sql_connector(sql_column=...) produced zero lineage with no warning. The extract_write_tables docstring example (FROM source) was itself unextractable by its sibling function. Both extractors now accept bare (0-dot) identifiers, filtered against a small keyword blocklist so subqueries, table functions, and row sources (SELECT, LATERAL, TABLE(...), VALUES, ...) are not mistaken for tables. Qualified names are never keyword-filtered. INSERT OVERWRITE is also recognized instead of capturing OVERWRITE as a table name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…g example The regex only matched {{var}} with no interior whitespace, so standard spaced template variables like {{ ds }} passed through untouched and broke downstream SQL parsing. It now tolerates whitespace inside the braces. The docstring example also claimed an output the function never produced; it now shows the actual result, and a doctest-shaped test pins it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per the repo's boundary rule, examples in tests use generic names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Six tests imported pandas mid-test with no importorskip (4 sample-model factory tests in test_datasets, plus one feature-names test each in test_introspect/test_sklearn and test_lightgbm). With sklearn/xgboost/ lightgbm installed but pandas absent — a combination no extra rules out, since pandas is only pulled in via the snowflake extra — the suite hard-failed instead of skipping. All six now importorskip pandas like every other optional dependency in the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The excel extra declared openpyxl>=3.1, but nothing in src/ or tests/ has imported openpyxl since the scanner module was removed (post-0.7.7). Drop the extra and its row in the installation guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Both factories accepted any object as backend — passing a Ledger (or a bare database path) by mistake failed deep inside the first request with a confusing AttributeError. A shared validate_backend() guard now raises a clear TypeError at construction, with targeted hints for the two common mistakes (a Ledger instance, a path string). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

sql_connector addresses row values by column name, but a connection returning raw DB-API tuple rows (e.g. default sqlite3) crashed with an opaque 'TypeError: tuple indices must be integers or slices, not str' deep inside row mapping. discover() now checks each row is a mapping and raises a TypeError that names the requirement and the fix (dict row_factory / DictCursor). Docs: the connectors guide and the factory docstring now state the dict-row requirement with a sqlite3 row_factory example. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

model-ledger export --output described an 'Output directory' and the success message read 'exported to pack_out/', but the command writes one HTML file at exactly the given path (previously extensionless by default). Help now says 'Output file path', the default gained a proper .html extension, and the success message prints the real artifact path. A richer artifact layout (per-format extensions or a real directory) is a deliberate CLI change deferred to a later release. CLI test asserts the help text and success message match the artifact actually produced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…rsion CHANGELOG.md stopped at v0.7.9 while the changelog URL is advertised in pyproject and the README. Backfills v0.7.10 (#32, #33), v0.7.11 (#34), and v0.7.12 (#35) from the release history, adds the v0.7.13 section for this fix batch, and bumps the package version to 0.7.13. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The trace tool computed each node's depth from its index in the flat transitive dependency list (depth = total - idx), so any model with N transitive upstreams rendered as an N-deep linear chain regardless of the graph's actual shape. Downstream traversal had the same fabrication. Traversal is now an explicit breadth-first walk that records each node's actual level (shortest edge distance from the traced model, one entry per node at its minimum depth) and terminates at input.depth, so depth-bounded queries do depth-bounded work instead of filtering after a full traversal. Regression fixture: 19-node fan-out DAG over 8 uneven levels with a diamond and a shortcut edge; 6 new tests fail on the old implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…le INSERT OVERWRITE TABLE Bare-identifier acceptance turned every WITH-clause binding into a phantom read table — dbt-style multi-CTE SQL reported its own CTE names as lineage inputs, and a CTE sharing a real table's name created spurious cross-model dependency edges. WITH-clause names (including nested and RECURSIVE forms, with optional column lists) are now parsed and subtracted from extracted reads; DELETE FROM targets are likewise excluded (a delete target is not a read). Write extraction now recognizes the Hive/Spark form INSERT OVERWRITE TABLE t, which the keyword filter previously dropped. Docstrings document the keyword-blocklist tradeoff: a bare table genuinely named a filtered keyword must be schema-qualified to extract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…'s 7-day default The changelog tool defaults since = now - 7 days when no bounds are given, and HttpLedgerBackend.list_snapshots called GET /changelog without one — so everything built on it was silently wrong for any model whose events are older than a week: get_snapshot reconstruction returned None, latest_snapshot returned None (Ledger.tag raised ModelNotFoundError on a healthy month-old model), the record tool's idempotency check missed the registration event, and the platform query filter returned total=0 for a known platform. list_snapshots now passes an explicit all-time since bound. Also cache the name mapping for hashes handed out by list_models: ModelSummary omits created_at, so those hashes are per-call placeholders; without the mapping, get_model/list_snapshots could not resolve them even for fresh events, which broke batch_platforms (and with it the platform filter) from any fresh client. Regression tests age server-side events 30 days and drive a cache-free second client through all four behaviors, plus the fresh-event hash-resolution case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Over HttpLedgerBackend the caller's registration payload was silently dropped: save_model posted payload={}, and the SDK's follow-up append_snapshot(registered) — the only carrier of the payload — was exactly the request the redundant-registration skip suppressed. Tier and actor were lost the same way (every remote registration landed as tier="unclassified", actor="user"). Ledger.register() now dispatches to an optional backend register_model hook when the backend defines one; HttpLedgerBackend implements it as a single POST /record carrying owner, model_type, purpose, tier, actor, and the caller's payload, then adopts the server's canonical hash as before. Backends without the hook keep the existing two-step save_model + append_snapshot path unchanged. RecordInput gains an optional tier field (REST body and MCP tool arg, additive); the record tool passes it through to register(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The dict-row guard used hasattr(row, "get") as a proxy for "addressable by column name", which rejected the stdlib's own mapping-style row type: sqlite3.Row supports keys() and row["col"] but has no .get(), so configurations that discovered nodes on 0.7.12 started raising the tuple-row TypeError. Rows with keys() are now materialized to dicts; only true positional rows are rejected. The docs example now recommends row_factory = sqlite3.Row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ull-protocol isinstance The runtime_checkable isinstance check demanded all 14 protocol method names, so a handwritten partial backend (no list_snapshots_before or tag methods) that served create_app() traffic fine on 0.7.12 died with TypeError at construction — a compatibility break for third-party backends. The guard now rejects the two intended mistakes (a Ledger instance, a path string/PathLike) with the same hints and otherwise requires only a core-method subset, so previously-working partial backends keep constructing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The dispatch regression tests importorskipped numpy/sklearn/xgboost/ lightgbm, and neither CI nor the default dev install carries them — all four tests skipped everywhere, leaving the dispatch fix with zero executed coverage in the release pipeline. Add a stub-module layer: minimal sklearn/xgboost/lightgbm modules with the real inheritance shape (wrapper estimators subclass BaseEstimator) installed via monkeypatch, exercising registry.find() and SklearnIntrospector.can_handle in every environment, plus a __module__ = None safety case. With the can_handle fix reverted, the stub tests fail in a bare venv (verified). The real-library tests remain and still run where the ML extras are installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…lt filename Removing the excel extra broke `model-ledger[excel]` pins within a patch version (uv hard-errors on unknown extras), and changing the export default artifact from audit_pack to audit_pack.html broke scripts consuming the old default path. Restore both: the extra is marked deprecated (removal planned for 0.8.0) and the export help text keeps the corrected single-file wording. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…distinct models; public Ledger.backend Three review notes: - RecordInput.model_name rejects whitespace-only values (min_length=1 let " " register a near-invisible model); REST returns 422. - TraceOutput.total_nodes counts distinct models — a node reachable both upstream and downstream with direction="both" (e.g. a 2-cycle) was counted twice. - Ledger gains a public read-only backend property; tool functions use it instead of reaching into the private _backend attribute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The get_snapshot comment implied rebuilt snapshots always recompute the identity the server holds; that is true only for server-minted events. Snapshots created client-side by Ledger.record() carry a client timestamp, so their hashes are process-local (cache-resolvable only) — say so explicitly. CHANGELOG: fold the review fix round into the v0.7.13 section (full changelog history over HTTP, registration fidelity, CTE/DELETE exclusions, sqlite3.Row, duck-typed backend validation, whitespace-name rejection, distinct total_nodes, stub-module dispatch coverage), add a compatibility note deferring the excel-extra removal and strict backend validation to 0.8.0, and drop the claims the review refuted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2e058a7c2

ℹ️ 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".

chatgpt-codex-connector · 2026-07-20T22:43:14Z

+                owner=m.get("owner") or "unknown",
+                model_type=m.get("model_type") or "unknown",
+                tier="unclassified",
+                purpose="",


Make HTTP tier reads match the newly persisted tier

When the remote backend now registers a model with the caller's tier, a fresh HTTP client still reads the model back as tier="unclassified" because list_models() reconstructs ModelRef from /query with this hard-coded value; get_model_by_name() has the same hard-code via /investigate. In an HTTP-backed workflow, registering tier="high" and then listing/getting from another process silently downgrades the tier, so the new registration fidelity is not actually round-tripped to SDK users.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-20T22:43:14Z

+    """
+    backend = ledger.backend
+
+    models_all = ledger.list(**filters)


Keep purpose text matches when platform is filtered

When input.platform is set, this path fetches all structurally matching models without passing input.text to ledger.list() and then does the text match locally. That works only for backends whose ModelRef includes purpose; HttpLedgerBackend.list_models() rebuilds refs from /query summaries with purpose="", so an HTTP-backed query such as platform="mlflow", text="scoring" misses a model whose purpose is “Credit scoring” unless its name also contains the text. The non-platform path still pushes text to /query, so this regresses the remote SDK/API context this change targets.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-20T22:43:14Z

+                "model_name": self._resolve_name(snapshot.model_hash),
                "event": snapshot.event_type,
                "payload": snapshot.payload,
                "actor": snapshot.actor,


Forward snapshot source on HTTP record calls

For Ledger(HttpLedgerBackend(...)), Ledger.record(..., source="mlflow") builds a Snapshot with source set, but this POST only sends the model name, event, payload, and actor. The server therefore stores source=None; because platform resolution falls back to snapshot.source when a discovered payload has no platform, HTTP-recorded discovered events that rely on source cannot be found by QueryInput(platform=...).

Useful? React with 👍 / 👎.

vigneshnarayanaswamy and others added 22 commits July 19, 2026 16:47

test: use generic identifiers in DataPort case-normalization test

a0e096c

Per the repo's boundary rule, examples in tests use generic names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Jul 20, 2026

View reviewed changes

vigneshnarayanaswamy merged commit 93be51a into main Jul 21, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: 0.7.13 batched fixes (HTTP backend integrity, query platform filter, trace depth, SQL lineage extraction)#36

fix: 0.7.13 batched fixes (HTTP backend integrity, query platform filter, trace depth, SQL lineage extraction)#36
vigneshnarayanaswamy merged 22 commits into
mainfrom
vigneshn/fix-batch-0.7.13

vigneshnarayanaswamy commented Jul 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

vigneshnarayanaswamy commented Jul 20, 2026

Summary

Fixes

Behavior notes

Testing

Compatibility

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant