Skip to content

Prepare native ingestion portability - #91

Merged
danielmawhirter merged 1 commit into
mainfrom
dm-platform-native
Nov 3, 2026
Merged

danielmawhirter merged 1 commit into
mainfrom
dm-platform-native

Conversation

@danielmawhirter

@danielmawhirter danielmawhirter commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Prepare native ingestion portability through platform-specific file I/O, executable resolution, process handling, local URIs and artifact ownership. Reject unsupported Windows profiling before modifying published output. This is preparatory runtime work: native Windows remains experimental and CI-only, with no native Windows release/support commitment. Existing optional-Faiss metadata is unchanged.

Base main; 22 files. Merge #91 → #140 → #92. Windows release support uses WSL2 with the Linux x86-64 wheel.

Head 6f52003cd86e9c68989975499ef4276243c0e0e5.

Change map

Maintained file-by-file map.

Testing Done

Rebased onto main 836e343c; all 19 stack commits are signed. Local checks passed: 188 packaging/workflow tests plus four subtests, 94 core tests, 158 datamodule tests (one required a subprocess-permission rerun), 656 Rust tests (two ignored), 250-document validation, artifact manifests, 410-component notices and flake8/isort. The workflow fixture now uses local pathlib operations instead of application dependencies; all 156 tests in the exact minimal repository-check environment passed without pydantic.

Fresh run 41570848349 at 8f5471712d5b36312b1ec899166da409ff9b1454 completed successfully: four producers, 144 dependency resolutions, twelve CPU reload jobs and the inherited Linux x86-64 GPU gate (eight tasks) passed. Release attachment and Kitmaker publication were skipped. The final stack retains both release gates; native Windows remains CI-only. No ARM/Windows GPU coverage or package publication is claimed.

Separate automatic connector checks fail only two restricted BigQuery tests missing GCP_BIGQUERY_NO_TABLE_SERVICE_ACCOUNT and GCP_BIGQUERY_NO_JOB_SERVICE_ACCOUNT; 1,911 other connector tests passed on each PR. That credential configuration is owned elsewhere and is not fixed here.

Hotfix Context

Release preparation.

Checklist

  • Breaking changes listed (none intended).
  • Examples and documentation updated.
  • Focused tests added or updated.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
Walkthrough

The changes add Windows support across Rust DiskGraph and Python components. They add platform-specific positioned I/O, file and directory syncing, disk-capacity queries, and memory-advice behavior. The Python runtime selects the Windows ingest executable and handles progress reads and unsupported resource limits. Filesystem URI generation and artifact-manifest paths now account for Windows paths. Tests cover these platform-specific behaviors and path handling.

Sequence Diagram(s)

sequenceDiagram
  participant Runner as _run_with_heartbeat
  participant Child as ingest subprocess
  participant Capture as output capture handle
  participant ProgressReader as progress reader handle
  Runner->>Capture: Open writable output handle
  Runner->>ProgressReader: Open separate reader handle
  Child->>Capture: Write subprocess output
  Runner->>ProgressReader: Read output for progress forwarding
Loading
Merge Risk: Moderate · previous review 4e910

On Windows, requesting CPU profiling during ingest fails only after the existing output has been invalidated or removed, so a previously published graph can be lost. The test and dev extras of kumo-core also still require a GPU Faiss release that has no Windows, macOS or ARM64 Linux wheel. Reject profiling before any cleanup and add platform markers to those extras before merging.

Pre-merge checks: Passed 2 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Change Map Design Review Inconclusive The required change-map comment is unavailable. The supplied PR description links to a file-by-file map, but it does not include the comment beginning <!-- kumo-pr-change-map -->; the checkout conta… Provide the exact PR conversation comment beginning <!-- kumo-pr-change-map -->, or an authorized export of that comment. Then assess each mapped change against the linked problem, existing behavior, verification, and the Windows profilin…
Passed Passed checks: 2 passed
Check name Status Explanation
Pr Size Passed The pull request changes 22 files. The limit is 40 files, so the PR size check passes.
Public Resource Access Passed No added or modified external URL was found. The new package references are tempfile = "3" at packages/diskgraph/diskgraph-fs/Cargo.toml:29 and windows-sys = "0.61.2" at `packages/diskgraph/disk…
Full details: Public Resource Access

Explanation

No added or modified external URL was found. The new package references are tempfile = "3" at packages/diskgraph/diskgraph-fs/Cargo.toml:29 and windows-sys = "0.61.2" at packages/diskgraph/diskgraph-ingest/Cargo.toml:73; anonymous crates.io API requests returned HTTP 200 for the locked versions tempfile 3.27.0 and windows-sys 0.61.2. The added ingest.exe path is a locally generated build artifact, not an external resource.

Full details: Change Map Design Review

Explanation

The required change-map comment is unavailable. The supplied PR description links to a file-by-file map, but it does not include the comment beginning &lt;!-- kumo-pr-change-map --&gt;; the checkout contains only the review guidance, not the comment body. The authoritative diff covers 22 files and 723 additions, so the description and code cannot establish the map's per-change rationale, smallest sufficient changes, focused verification, or stopping condition. Direct inspection also found a material design point the missing map should explain: run_phase_with_marker_payload invalidates publication before Windows profile_phase rejects the request (lib.rs:13031-13049).

Resolution

Provide the exact PR conversation comment beginning &lt;!-- kumo-pr-change-map --&gt;, or an authorized export of that comment. Then assess each mapped change against the linked problem, existing behavior, verification, and the Windows profiling failure path.

Finishing Touches
Fix failing CI checks
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9bd9fef and 4e91036.

Files ignored due to path filters: 1
  • packages/diskgraph/Cargo.lock is excluded by !**/*.lock, !**/*.lock
Files selected for processing: 3
  • packages/diskgraph/diskgraph-ingest/src/lib.rs
  • packages/kumo-core/src/kumo_core/diskgraph/lightning_data.py
  • test/train/datamodule/test_diskgraph_lightning_data.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@danielmawhirter

danielmawhirter commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Change map

Prepare native ingestion portability through platform-specific file I/O, executable resolution, process handling, local URIs and artifact ownership. Reject unsupported Windows profiling before modifying published output. This is preparatory runtime work: native Windows remains experimental and CI-only, with no native Windows release/support commitment. Existing optional-Faiss metadata is unchanged.

Base main; 22 files. Merge #91 → #140 → #92. Windows release support uses WSL2 with the Linux x86-64 wheel.

Head 6f52003cd86e9c68989975499ef4276243c0e0e5.

File Function / scope What changed Why
packages/diskgraph/Cargo.lock Cargo dependency graph Records target-specific Windows API dependencies. Keeps native builds reproducible.
packages/diskgraph/diskgraph-fs/Cargo.toml Crate dependencies Adds the test fixture dependency. Tests actual file-offset reads.
packages/diskgraph/diskgraph-fs/src/cloud_transfer.rs Positioned file reads Selects Unix or Windows file APIs and tests offset and EOF behavior. Windows lacks the Unix FileExt import.
packages/diskgraph/diskgraph-index/src/graph.rs Completed-shard file/directory flushing Opens Windows flush handles with write access and directory backup semantics; adds flush/content/missing-path tests. Fixes observed read-only-handle AccessDenied while retaining fatal flush errors and the existing cleanup gate; Windows directory durability remains unverified.
packages/diskgraph/diskgraph-index/src/pkmap.rs Memory-map advice Limits Unix advice calls to Unix targets. Advice is an optimization unavailable on Windows.
packages/diskgraph/diskgraph-index/src/reader.rs Memory-map advice Limits Unix advice calls to Unix targets. Keeps graph reads buildable on Windows.
packages/diskgraph/diskgraph-ingest/Cargo.toml Target-specific dependencies Restricts profiling dependency to Unix and enables Windows disk-space APIs. Native ingestion needs platform-appropriate dependencies.
packages/diskgraph/diskgraph-ingest/src/disk_manager/mod.rs Disk capacity queries Adds Windows volume-capacity queries and tests. Unix statvfs is unavailable on Windows.
packages/diskgraph/diskgraph-ingest/src/disk_manager/path_safety.rs Platform-safe graph keys/versions Rejects colon-bearing Windows components, tests graph-key and version rejection, preserves Unix timestamp components, and gates the Unix symlink test. Windows drive prefixes can discard the owned root and colons can select alternate streams.
packages/diskgraph/diskgraph-ingest/src/lib.rs Profiling, ingestion entry validation and map advice Gates Unix APIs and rejects Windows profiling before config reads or publication/output mutations; tests existing markers/sentinel preservation and absent new output. Unsupported profiling must fail before invalidating a published graph.
packages/diskgraph/diskgraph-ingest/src/mapping.rs Mapping advice Applies Unix map advice only on Unix. Preserves mapped ingestion on Windows.
packages/diskgraph/diskgraph-ingest/src/ts_grouping.rs Positioned reads/writes and Unix file-limit test Adds target-specific exact positioned I/O and regression coverage; gates the Unix low-file-limit test to Unix. Preserves parallel merge behavior and allows native test compilation.
packages/diskgraph/pyproject.toml Maturin wheel contents Explicitly includes the staged ingest.exe binary. The native package needs its ingestion executable.
packages/diskgraph/third_party/artifact-manifest.json Generated artifact graphs Regenerates source and lock fingerprints against current main while retaining target-specific dependency graphs. Keeps provenance consistent with merged native RDL/history and index-clock changes.
packages/kumo-core/src/kumo_core/diskgraph/lightning_data.py Linux pidfd initialization Loads libc only on Linux and asserts its presence in syscall fallback paths. Native Windows source installation succeeded but importing the engine failed at unconditional CDLL(None).
test/train/datamodule/test_diskgraph_lightning_data.py Platform and syscall regression tests Checks Windows/macOS avoid loading libc and exercises Linux pidfd fallback open/signal behavior. Prevents the demonstrated import blocker while preserving Linux process handling.
packages/kumo-core/src/kumo_core/io/filesystem.py Local filesystem URI reconstruction Emits file:/// drive-absolute Windows URIs for listing and recursive-copy results while preserving other filesystem protocols. Pandas interpreted file://C:/ validation output as a host rather than a local drive, blocking metrics loading after training.
packages/kumo-core/tests/test_portable_io.py Listing/copy URI regression coverage Covers Windows drive paths, unaffected local path forms, copy results, and local/memory CSV round trips. Verifies the URI contract needed to read generated validation results without changing remote protocols.
packages/kumo-core/src/kumo_core/train/artifact_manifest.py Ownership coordinates and manifest path construction Preserves literal remote-key backslashes, normalizes local/file separators and Windows drive-letter case, and uses URI-aware joining at all 13 manifest producer/consumer sites. Prevents ownership escapes and allows POSIX-produced remote manifests to validate on Windows without changing native filesystem joins or directory-case boundaries.
packages/kumo-core/tests/test_prediction_manifest_storage.py Ownership and cross-platform manifest regressions Tests remote-key boundaries, exact directory case, URI/local join selection, and full in-memory shard/selection publication and loading across simulated Windows/POSIX hosts, including literal-backslash roots. Covers the reviewed ownership flaws and the remote-manifest validation failure through the complete producer/consumer path.
packages/kumo-core/src/kumo_core/diskgraph/runner.py Ingest resolution, limits, progress forwarding Resolves ingest.exe on Windows, handles unavailable limits explicitly, and reads progress through an independent handle. Windows lacks resource and pread; progress reads must not change the child's write offset.
packages/kumo-core/tests/test_diskgraph_runtime.py Native runtime regression coverage Covers binary selection, unsupported limits, progress offsets, and actual subprocess output. Verifies runtime behavior while preserving upstream real-ingest tests.

Focused verification: Rebased onto main 836e343c; all 19 stack commits are signed. Local checks passed: 188 packaging/workflow tests plus four subtests, 94 core tests, 158 datamodule tests (one required a subprocess-permission rerun), 656 Rust tests (two ignored), 250-document validation, artifact manifests, 410-component notices and flake8/isort. The workflow fixture now uses local pathlib operations instead of application dependencies; all 156 tests in the exact minimal repository-check environment passed without pydantic.

Fresh run 41570848349 at 8f5471712d5b36312b1ec899166da409ff9b1454 completed successfully: four producers, 144 dependency resolutions, twelve CPU reload jobs and the inherited Linux x86-64 GPU gate (eight tasks) passed. Release attachment and Kitmaker publication were skipped. The final stack retains both release gates; native Windows remains CI-only. No ARM/Windows GPU coverage or package publication is claimed.

Separate automatic connector checks fail only two restricted BigQuery tests missing GCP_BIGQUERY_NO_TABLE_SERVICE_ACCOUNT and GCP_BIGQUERY_NO_JOB_SERVICE_ACCOUNT; 1,911 other connector tests passed on each PR. That credential configuration is owned elsewhere and is not fixed here.

Stopping condition: reviewed portable runtime, strict wheel assembly and installed-wheel CPU qualification with WSL2-only Windows release support. No native Windows publication, GPU or multi-worker support claim. No upload, release or version bump is authorized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (3)

🟠 Major · Reject colon-bearing components on Windows. · path_safety.rs:17-25

packages/diskgraph/diskgraph-ingest/src/disk_manager/path_safety.rs:17-25
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Reject colon-bearing components on Windows.

safe_index_path promises a path under data_root, but validate_path_component allows :. On Windows, Path::join discards the base when the component has a prefix without a root. Therefore, C:evil can discard data_root. Colon-bearing components can also be interpreted as stream syntax.

Update the validator and keep the platform-specific test expectations aligned.

Suggested fix
     if name.contains('/') || name.contains('\\') {
         bail!(
             "path component must not contain path separators: {:?}",
             name
         );
     }
+    #[cfg(windows)]
+    if name.contains(':') {
+        bail!("path component must not contain ':' on Windows: {:?}", name);
+    }
@@
-        assert!(validate_path_component("2025-03-27T12:00:00Z").is_ok());
+        #[cfg(not(windows))]
+        assert!(validate_path_component("2025-03-27T12:00:00Z").is_ok());
     }
 
+    #[cfg(windows)]
+    #[test]
+    fn test_rejects_windows_colon_components() {
+        assert!(validate_path_component("C:evil").is_err());
+        assert!(validate_path_component("2025-03-27T12:00:00Z").is_err());
+    }
🟡 Minor · Gate merge_open_files_do_not_scale_with_shards with #[cfg(unix)]. · ts_grouping.rs:725-744

packages/diskgraph/diskgraph-ingest/src/ts_grouping.rs:725-744
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Gate merge_open_files_do_not_scale_with_shards with #[cfg(unix)].

This test uses the Unix-only libc::rlimit, libc::setrlimit, and libc::RLIMIT_NOFILE APIs. Without the gate, cargo test -p diskgraph-ingest cannot compile this test on Windows.

Suggested fix
+    #[cfg(unix)]
     #[test]
     fn merge_open_files_do_not_scale_with_shards() {
🟡 Minor · Mark Faiss dependencies in the package-local test and dev extras. · pyproject.toml:80

packages/kumo-core/pyproject.toml:80
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Mark Faiss dependencies in the package-local test and dev extras.

The normal workspace setup uses kumo-core[faiss-cpu], not these package-local extras. However, direct Windows installation of kumo-core[test] or kumo-core[dev] can fail because both still request unmarked faiss-gpu==1.14.3.

🐛 Suggested fix
 test = [
-  "faiss-gpu==1.14.3",
+  "faiss-cpu==1.14.2; sys_platform == 'win32'",
+  "faiss-gpu==1.14.3; sys_platform != 'win32'",
   "packaging>=24",
   "pytest>=8",
   "pytest-cov>=5",
 ]
 dev = [
-  "faiss-gpu==1.14.3",
+  "faiss-cpu==1.14.2; sys_platform == 'win32'",
+  "faiss-gpu==1.14.3; sys_platform != 'win32'",
   "packaging>=24",
   "pytest>=8",
   "pytest-cov>=5",
   "ty==0.0.23",
 ]
🟡 Other comments: 1
packages/kumo-core/src/kumo_core/train/artifact_manifest.py: 1

101-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Compare ordinary Windows paths without case sensitivity.

On a default Windows directory, C:/work/predictions and c:/work/predictions/part.parquet refer to the same drive and a valid descendant. This branch preserves case, so require_owned_path rejects the file. Normalize case for local Windows coordinates without changing remote object-key comparison. Account for directories configured as case-sensitive. (learn.microsoft.com)


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/kumo-engine/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: bab8a042-bd3d-47f6-af62-4769c301aaf5
📥 Commits

Reviewing files that changed from the base of the PR and between bdb6294 and 2bdb206.

⛔ Files ignored due to path filters: 2
  • packages/diskgraph/Cargo.lock is excluded by !**/*.lock, !**/*.lock
  • uv.lock is excluded by !**/*.lock, !**/*.lock
📒 Files selected for processing: 24
  • packages/diskgraph/diskgraph-fs/Cargo.toml
  • packages/diskgraph/diskgraph-fs/src/cloud_transfer.rs
  • packages/diskgraph/diskgraph-index/src/graph.rs
  • packages/diskgraph/diskgraph-index/src/pkmap.rs
  • packages/diskgraph/diskgraph-index/src/reader.rs
  • packages/diskgraph/diskgraph-ingest/Cargo.toml
  • packages/diskgraph/diskgraph-ingest/src/disk_manager/mod.rs
  • packages/diskgraph/diskgraph-ingest/src/disk_manager/path_safety.rs
  • packages/diskgraph/diskgraph-ingest/src/lib.rs
  • packages/diskgraph/diskgraph-ingest/src/mapping.rs
  • packages/diskgraph/diskgraph-ingest/src/ts_grouping.rs
  • packages/diskgraph/pyproject.toml
  • packages/diskgraph/third_party/artifact-manifest.json
  • packages/kumo-core/pyproject.toml
  • packages/kumo-core/src/kumo_core/diskgraph/lightning_data.py
  • packages/kumo-core/src/kumo_core/diskgraph/runner.py
  • packages/kumo-core/src/kumo_core/io/filesystem.py
  • packages/kumo-core/src/kumo_core/train/artifact_manifest.py
  • packages/kumo-core/tests/test_diskgraph_runtime.py
  • packages/kumo-core/tests/test_portable_io.py
  • packages/kumo-core/tests/test_prediction_manifest_storage.py
  • packages/kumo-engine/pyproject.toml
  • packages/kumo-engine/setup.py
  • test/train/datamodule/test_diskgraph_lightning_data.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

Comment thread packages/kumo-core/pyproject.toml Resolved
@danielmawhirter

Copy link
Copy Markdown
Collaborator Author

Addressed the remaining findings: Windows graph keys/versions reject colon-bearing drive/stream syntax; the low-file-limit test is Unix-only; test/dev Faiss extras select GPU on Linux and CPU elsewhere; local/file Windows ownership checks normalize only drive-letter case and retain exact directory case. Local checks passed: 40 Python ownership/I/O tests, 26 Rust index tests, 10 path-safety tests, and eight timestamp-grouping tests. PR92 adds a native Windows Rust path-safety test step for the Windows-only colon cases; that step awaits current-head CI. Historical Windows/WSL smoke passes predate these fixes; current-head qualification is pending.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟠 Major · Use POSIX separators for URI ownership roots. · artifact_manifest.py:452

packages/kumo-core/src/kumo_core/train/artifact_manifest.py:452
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use POSIX separators for URI ownership roots.

A POSIX-produced shard can store a remote root such as s3://bucket/root/_attempts/id. On Windows, osp.join creates s3://bucket/root\_attempts. URI canonicalization preserves that backslash, so require_owned_path rejects the valid root.

🐛 Suggested fix
-                    root, osp.join(self.canonical_output_root, "_attempts"))
+                    root, posixpath.join(self.canonical_output_root, "_attempts"))

ℹ️ Review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 2bdb206 and d6ace99.

⛔ Files ignored due to path filters: 2
  • packages/diskgraph/Cargo.lock is excluded by !**/*.lock, !**/*.lock
  • uv.lock is excluded by !**/*.lock, !**/*.lock
📒 Files selected for processing: 6
  • packages/diskgraph/diskgraph-index/src/reader.rs
  • packages/diskgraph/diskgraph-ingest/src/disk_manager/path_safety.rs
  • packages/diskgraph/diskgraph-ingest/src/ts_grouping.rs
  • packages/kumo-core/pyproject.toml
  • packages/kumo-core/src/kumo_core/train/artifact_manifest.py
  • packages/kumo-core/tests/test_prediction_manifest_storage.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@danielmawhirter

Copy link
Copy Markdown
Collaborator Author

Addressed the URI ownership-root finding in review 5619620260. All 13 manifest producer/consumer joins now use POSIX separators for URIs while retaining native filesystem joining for local paths. This also fixes the earlier generation/control-root validation checks; remote object-key backslashes remain literal.

The Windows-simulated regression reproduces the original misbound-generation failure with the old join behavior. The new tests exercise complete shard/selection publication and loading across simulated Windows/POSIX hosts, including literal-backslash roots. All 52 focused tests passed (38 core, 14 hosted), plus changed-file flake8/isort checks. Current-head remote platform qualification remains pending.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9bd9fef and 4e91036.

Files ignored due to path filters: 1
  • packages/diskgraph/Cargo.lock is excluded by !**/*.lock, !**/*.lock
Files selected for processing: 3
  • packages/diskgraph/diskgraph-ingest/src/lib.rs
  • packages/kumo-core/src/kumo_core/diskgraph/lightning_data.py
  • test/train/datamodule/test_diskgraph_lightning_data.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@danielmawhirter
danielmawhirter temporarily deployed to gha-kumo-workflow-tests October 30, 2026 16:48 — with GitHub Actions Inactive
@danielmawhirter danielmawhirter changed the title Support native Windows engine ingestion and CPU Faiss Prepare native ingestion portability Oct 30, 2026
devanj08 added a commit that referenced this pull request Oct 31, 2026
…#134)

## Summary

Add `Graph.visualize()` to inspect declared tables and relationships
directly in notebooks.
Standard and Compact layouts include light and dark themes, schema
details, relationship highlighting and optional sample values.
Export SVG, vector PDF or Markdown on the machine running Kumo Engine,
with selectable save commands in the notebook toolbar and a static SVG
fallback for saved HTML output.
Notebook display automatically uses environment-provided
anywidget/ipywidgets in Snowflake Workspace and the existing HTML path
elsewhere, without changing package requirements.

See the [change
map](#134 (comment))
for file-by-file rationale.

## What Happens and Why

Schema definitions alone make it difficult to inspect relationships
across multiple tables before training.
Hover-driven camera movement shifts rows beneath the pointer, while
fixed light colors make schema details unsuitable for dark notebook
displays.
Notebook host controls can also overlap graph actions placed at the top
right.
Snowflake Workspace blocks the scripted HTML path, saved Jupyter outputs
can strip scripts and inline SVG, and VS Code relocates executable
scripts outside their original graph element.

## The Fix

Render declared schemas in deterministic Standard/Compact layouts with
stationary hover details, coordinated light/dark styling, manual camera
controls, optional samples and sample-free SVG/PDF/Markdown exports.

### Public API

Options and examples:

```python
graph.visualize(
    path=None,             # str, pathlib.Path, or None
    show_cols=False,
    show_samples=False,
    layout="auto",         # "auto", "standard", or "compact"
)
```

| Option | Behavior |
| --- | --- |
| `path=None` | Return SVG text with interactive notebook display
support, without writing a file. |
| `path="graph.svg"` | Write UTF-8 SVG of the initial layout, including
the displayed columns. |
| `path="graph.pdf"` | Write the vector diagram and complete paginated
schema/connection index. |
| `path="graph.md"` or `path="graph.markdown"` | Write a Mermaid
relationship diagram and all-column Markdown schema tables. |
| `show_cols=False` | Display PK, FK and declared time (`TS`) and
end-time (`TE`) columns. |
| `show_cols=True` | Include other columns, up to 10 rows per table, or
5 above 10 tables; key/time/end-time columns always remain visible even
above that limit. |
| `show_samples=False` | Read no source data. |
| `show_samples=True` | Add up to six non-null samples per visible
column to notebook tooltips from a preview of up to 30 rows per table. |
| `layout="auto"` | Choose Standard or Compact from graph structure,
independently of viewport size. |
| `layout="standard"` | Select the layered relationship layout. |
| `layout="compact"` | Select the compact arrangement. |
| Returned SVG: `.as_widget()` | Optional explicit widget conversion;
`Graph.visualize()` still returns SVG text and selects the notebook
transport at display time. |

```python
from pathlib import Path
from IPython.display import display

# Notebook display; no source-data reads by default.
graph.visualize()
graph.visualize(show_cols=True)
graph.visualize(show_samples=True)
graph.visualize(show_cols=True, show_samples=True, layout="compact")

# The same API automatically uses widget MIME in Snowflake Workspace.
display(graph.visualize(show_cols=True))

# Explicit layout selection.
graph.visualize(layout="auto")
graph.visualize(layout="standard")
graph.visualize(layout="compact")

# Save on the machine running Engine, including a training machine.
graph.visualize("graph.svg", show_cols=True, layout="standard")
graph.visualize(path=Path("graph.pdf"), show_cols=True, layout="compact")
graph.visualize("graph.md")
graph.visualize("graph.markdown")

# The return value remains SVG text, including when a file is written.
view = graph.visualize()
svg_text = str(view)
```

File suffix matching is case-insensitive; any suffix other than `.pdf`,
`.md` or `.markdown` receives SVG.
Paths are relative to the current working directory unless absolute,
parent directories must already exist, and existing files are
overwritten.
PDF and Markdown indexes include every declared column regardless of
`show_cols`; Markdown uses the viewer's Mermaid layout rather than the
notebook arrangement, and its schema tables remain readable without
Mermaid support.
The Export commands button reveals `_ = graph.visualize("graph.pdf",
layout="standard")` first (using the selected layout) and `_ =
graph.visualize("graph.md")` below it; executing a command writes to the
notebook filesystem without a browser download or eager PDF generation.
The default HTML view follows browser appearance, the widget view
inherits a declared host color scheme when mounted, and SVG/PDF files
and the saved static image use the light palette with sample values
excluded.

### Notebook hosts and saved outputs

| Context | Display and persistence contract |
| --- | --- |
| Databricks, local Jupyter and other HTML hosts | `graph.visualize()`
or `display(graph.visualize())`; existing self-contained HTML path with
Databricks `nodarkmode`, no anywidget import or comms |
| Snowflake Workspace | The same `graph.visualize()` and
`display(graph.visualize())` automatically publish actual widget MIME
when the Workspace marker and active Snowflake notebook extension are
present; one cached model supports independent frontend views, with no
scripted HTML competing for display |
| Workspace packages | [Runtime 2.6+ preinstalls
anywidget/ipywidgets](https://docs.snowflake.com/en/user-guide/ui-snowsight/notebooks-in-workspaces/notebooks-in-workspaces-interactive-widgets#installation),
including the documented GPU 2.9 target; older runtimes need compatible
packages under their environment policy; missing or incompatible widget
packages produce a static graph and actionable diagnostic |
| Connector extras | `kumo-engine[snowflake]` adds connector
dependencies; package installation, connector choice and credentials do
not select the notebook renderer |
| Saved HTML output | Complete, sample-free SVG image in the
Python-requested initial layout plus **“Rerun this cell to activate
animation.”**; hidden interactive controls until initialization
succeeds; rerun once after upgrading and save to replace old output; no
saved browser camera or layout-toggle state |
| Widget restoration and trust | Snowflake widget state is not
persistent across sessions; HTML fallback does not repair a missing
widget model; VS Code Restricted Mode can refuse HTML/SVG entirely |
| File retrieval and fullscreen | PDF/Markdown commands save in the
notebook working directory for notebook-side retrieval; fullscreen
remains host-permission-dependent with a clear denial message |

### Mac and Windows compatibility

| Target | Scope and evidence |
| --- | --- |
| Mac/Windows browser connected to a hosted notebook | Native Engine
dependencies run on the kernel host; no client-side Engine port needed,
but actual macOS/Windows browser sessions remain untested |
| Native Apple Silicon Mac | [macOS support already merged in
#47](#47); matching macOS
wheel and native notebook smoke still required for this candidate; local
validation artifact is Linux-only |
| Native Windows | Public `Graph` import reaches the compiled DiskGraph
extension before rendering; [native portability
#91](#91) and [experimental
Windows wheel work #140](#140)
remain prerequisites, not support added by this visualization PR |
| Validation limits | Linux Chromium/Firefox/WebKit tests and
cross-platform resolver checks; no native Mac/Windows installation,
Safari-on-macOS or Windows browser certification; WSL is a separate
Linux target |

## Blast Radius

Dependency declarations and `uv.lock` are unchanged from the PR base;
graphs remain limited to 30 tables with valid declared schemas and no
self-relations, PDF glyphs unavailable in the bundled fonts use explicit
Unicode escapes, and notebook interaction depends on host
HTML/widget/fullscreen permissions.
Sample previews can run warehouse queries and incur costs, their row
limit does not bound bytes scanned, and saved notebook HTML or widget
state can contain sampled values even though file exports and the static
fallback image do not.

## Verification

The [graph/export
tests](https://github.com/NVIDIA/kumo-engine/blob/1d63aed16f8f36d24585e63460c63c03a9275921/packages/kumo-engine/test/test_graph_validation.py)
and [layout
tests](https://github.com/NVIDIA/kumo-engine/blob/1d63aed16f8f36d24585e63460c63c03a9275921/packages/kumo-engine/test/test_visualization_layout.py)
pass together (99 tests), including temporal-role retention beyond
column limits, actual PDF/Markdown exports, PDF badge-spacing checks
with bundled fonts, browser tooltip/visibility checks and scoped
lint/import/whitespace checks.
Prior verification covers the complete installed wheel, actual
IPython/kernel Workspace dispatch, Jupyter save/reopen, repeated-output
VS Code renderer replay and genuine Databricks strict/theme/fullscreen
checks; this end-time correction is checked against source without a new
wheel or hosted deployment; host replays do not certify authenticated
frontends or native Mac/Windows systems, and the existing
restricted-BigQuery credential failures remain reproduced on the [exact
base commit's CI
job](https://github.com/NVIDIA/kumo-engine/actions/runs/40782418127/job/123147227893).

---------

Co-authored-by: Haowei Yu <howyu@nvidia.com>
Victsm
Victsm previously approved these changes Nov 2, 2026

@Victsm Victsm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this exact head. The Linux native I/O paths retain the Unix operations; the portability changes and focused regressions look sound.

Fresh bounded Linux checks: 15 alternating pairs of cached positioned reads/writes (256 MiB read + 16 MiB write per operation) had median 45.24 ms base / 42.44 ms candidate. Across 21 pairs, the actual ingest-supervisor paths differed by approximately 0.2 ms per launch (64 KiB and 1 MiB output cases). Output checks matched. No material slowdown observed; these are changed-path microbenchmarks, not a full-ingestion or cold-storage claim.

Please resolve the merge conflict and clear required CI before merging. The restricted-BigQuery identity configuration failure is separate from these runtime changes.

Victsm
Victsm previously approved these changes Nov 3, 2026

@Victsm Victsm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed this head. The runtime diff matches the previously approved portability implementation apart from the regenerated manifest. Current platform qualification also covers the integrated stack. Approval is for the code; resolve the current merge conflict before landing.

Build and package DiskGraph ingestion on Windows; adapt file I/O, process handling, local file URIs, and prediction ownership paths. Reject unsupported Windows profiling before input reads or publication changes, and preserve Unix behavior.
@danielmawhirter
danielmawhirter merged commit fda914f into main Nov 3, 2026
122 of 123 checks passed
@danielmawhirter
danielmawhirter deleted the dm-platform-native branch November 3, 2026 18:42
danielmawhirter added a commit that referenced this pull request Nov 3, 2026
## Summary

Build Linux ARM64 and experimental native Windows wheels through the
shared assembly and native/legal verification pipeline, retaining Linux
x86-64 and macOS ARM64. Keep four CI build artifacts, but explicitly
allow only existing Linux x86-64 and macOS ARM64 wheels in release
attachment and publication. ARM64 release eligibility is deferred to
#92; native Windows stays disabled. Build checkout no longer persists
credentials.

Base `dm-platform-native`; 16 incremental files. Merge #91 → #140 → #92.
Windows release support uses WSL2 with the Linux x86-64 wheel.

Head `f87071eda8cf48f6cc138e65ca1857721e4a552b`.

## Change map

[Maintained file-by-file
map](#140 (comment)).

## Testing Done

Rebased onto main `836e343c`; all 19 stack commits are signed. Local
checks passed: 188 packaging/workflow tests plus four subtests, 94 core
tests, 158 datamodule tests (one required a subprocess-permission
rerun), 656 Rust tests (two ignored), 250-document validation, artifact
manifests, 410-component notices and flake8/isort. The workflow fixture
now uses local pathlib operations instead of application dependencies;
all 156 tests in the exact minimal repository-check environment passed
without pydantic.

Fresh [run
41570848349](https://github.com/NVIDIA/kumo-engine/actions/runs/41570848349)
at `8f5471712d5b36312b1ec899166da409ff9b1454` completed successfully:
four producers, 144 dependency resolutions, twelve CPU reload jobs and
the inherited Linux x86-64 GPU gate (eight tasks) passed. Release
attachment and Kitmaker publication were skipped. The final stack
retains both release gates; native Windows remains CI-only. No
ARM/Windows GPU coverage or package publication is claimed.


Separate automatic connector checks fail only two restricted BigQuery
tests missing `GCP_BIGQUERY_NO_TABLE_SERVICE_ACCOUNT` and
`GCP_BIGQUERY_NO_JOB_SERVICE_ACCOUNT`; 1,911 other connector tests
passed on each PR. That credential configuration is owned elsewhere and
is not fixed here.

## Hotfix Context

Release preparation.

## Checklist

- [x] Breaking changes listed (none intended).
- [x] Examples and documentation updated.
- [x] Focused tests added or updated.
danielmawhirter added a commit that referenced this pull request Nov 3, 2026
## Summary

Qualify same-run installed wheels on WSL2 and Linux ARM64 across Python
3.11–3.14, retaining experimental native Windows CPU checks. Extend the
same single-fit smoke with CPU bundle reload and prediction parity on
the identical population/snapshot; entity IDs, counts and values must
match (atol=1e-4), with no additional optimizer steps. Enable Linux
ARM64 release attachment and publication only behind installed-wheel
qualification. The three release artifacts are Linux x86-64 (also used
by WSL2), Linux ARM64 and macOS ARM64. Native Windows wheels remain
CI-only and are excluded from both release attachment and publication.

Base `dm-platform-wheels`; 9 incremental files. Merge #91 → #140 → #92.
Windows release support uses WSL2 with the Linux x86-64 wheel.

Head `3bac19078321174c7dcb1be01a1aff0b06291bfc`.

## Change map

[Maintained file-by-file
map](#92 (comment)).

## Testing Done

Rebased onto main `524a2ce5`. Local checks passed: 127 Python tests plus
four subtests, 105 Rust pybind tests, 35 runtime tests, 410-component
notice and artifact-manifest checks, 248-document validation,
flake8/isort, actionlint and workflow security. All 19 stack commits
have verified SSH signatures.

Fresh [run
41519588041](https://github.com/NVIDIA/kumo-engine/actions/runs/41519588041)
at `3bac19078321174c7dcb1be01a1aff0b06291bfc` passed four producers,
twelve installed-wheel CPU reload jobs and 144 ADLS-inclusive dependency
resolutions. Release attachment and Kitmaker were skipped; nothing was
published.

Separate connector CI still fails two tests missing
`GCP_BIGQUERY_NO_TABLE_SERVICE_ACCOUNT` and
`GCP_BIGQUERY_NO_JOB_SERVICE_ACCOUNT`; 1,875 other connector tests
passed. Credential configuration is owned elsewhere and is not fixed
here.

## Hotfix Context

Release preparation.

## Checklist

- [x] Breaking changes listed (none intended).
- [x] Examples and documentation updated.
- [x] Focused tests added or updated.

Release attachment and Kitmaker publication jobs were skipped in this
manual run; no package was published.

This branch was previously deployed

1 inactive deployment
gha-kumo-workflow-tests — 6f52003c Deployed Nov 3, 2026 by danielmawhirter via import #1326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants