Repository navigation
Add a generic agent-skills host with an access-aware preflight - #48
kumo-isaac wants to merge 22 commits into
Conversation
The kumo-data-science skill shipped only to Snowflake CoCo, and its "portable" core assumed Snowflake throughout. This adds a `generic` host for coding agents beside the user (Claude Code, Codex) that generates code for any of the six kumo.engine connectors. - Preflight §0: before inferring structure, the agent proves it can read source_scope with a read-only tool it already has. If not, it says what it checked and asks for access or for the structure, never for a credential. - hosts/generic/: adapter (source forms, access checks per system, destinations) and runtime (config cell with placeholders, connect step, destinations and read-back per connector). - Core no longer names a host; source and destination forms are host-defined. The SDK reference gains §3.4 (non-Snowflake connectors) and §9.2 (destinations by connector). - package_skill.py builds `generic`; tests check that the core names no host, that each package ships only its own host, and that every adapter covers what the core defers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Change Map Design Review |  |
The required PR conversation comment beginning <!-- kumo-pr-change-map --> is not available in the supplied review context or repository checkout. The checkout contains only the review guidance and … |
Provide the complete, current PR conversation comment beginning <!-- kumo-pr-change-map -->, including its Change map, linked problem or release requirement, focused verification, and stopping condition. Re-run the design review against t… |
-
Check name Status Explanation Pr Size The pull request changes 21 files in the review-scoped Git range. The limit is 40 files, so the PR size check passes. Public Resource Access No added or modified URL is present. The added package source references in agent-skills/skills/kumoai-reference/references/kumo_finetuning_sdk.md(lines 252–254 and 616–617) resolve to public sourc…Full details: Public Resource Access
-
Explanation
No added or modified URL is present. The added package source references in
agent-skills/skills/kumoai-reference/references/kumo_finetuning_sdk.md(lines 252–254 and 616–617) resolve to public source files inNVIDIA/kumo-engine; anonymous requests returned HTTP 200 for all nine referenced files. Added runtime package references also resolve through public PyPI entries. Internal repository paths exist in the reviewed tree. No inaccessible external resource reference was confirmed.
-
Full details: Change Map Design Review
-
Explanation
The required PR conversation comment beginning
<!-- kumo-pr-change-map -->is not available in the supplied review context or repository checkout. The checkout contains only the review guidance and the PR diff; it does not contain the Change map table, its linked problem, or its stated verification and stopping condition. The PR description and objectives summarize the change, but they do not provide the comment required by this check, so necessity, abstraction-layer placement, and coverage cannot be assessed reliably.Resolution
Provide the complete, current PR conversation comment beginning
<!-- kumo-pr-change-map -->, including its Change map, linked problem or release requirement, focused verification, and stopping condition. Re-run the design review against that comment and the reviewed diff.
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Found by the generic-host codegen eval (kumo-ai/kumo_skill_dev, eval/generic/): 32 of 38 files notebooks and 19 of 38 S3 notebooks named tables `x.parquet`, which carries the suffix into the graph and the PQL; 19 and 18 rooted the output connector at the confirmed directory and wrote `table_name='predictions'` one level below it. - adapter §3: name file and object-store tables without their suffix. - runtime §2: the same at the connect step. - runtime §3: root the output connector at the destination's parent and name the last segment in output_paths, so predictions land at exactly the confirmed path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
…rectory GPU runs of the generic files notebooks (kumo_skill_dev eval/generic/run_gpu.py) failed every static task with a staged population: pyarrow does not create the population directory, and kumo-engine 1.0.0 rejects a population without TIMESTAMP when the entity table is timestamped (kumo_finetuning_sdk.md §7.1 already says so; runtime §3.1 staged ENTITY1 only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
The rerun's nov-sales-regression notebooks still staged ENTITY1 only: the example's 'drop if there is none' read as optional. Decide from the entity Table's time_column. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
The Databricks GPU runs of the generic notebooks staged the static population with a wait_timeout above Databricks' 50 s limit and without TIMESTAMP. §3.1 now covers Databricks: CREATE TABLE AS through the confirmed SQL warehouse, wait <= 50 s, poll to completion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
GPU reruns staged TIMESTAMP from a DATE column (booked_date); kumo-engine 1.0.0 reads only a timestamp or int64 anchor and rejects date32. Cast it, in pyarrow and in the Databricks SQL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
SageMaker runs of the generic S3 notebooks: 5 of 38 added an AWS_REGION or AWS_PROFILE placeholder, which stops the run at the config guard; on a job role there is no profile to fill. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
SageMaker runs: 3 of 38 generic S3 notebooks still invented an AWS_REGION placeholder after feefa6d, which stops them at the config guard. The SDK reference listed region= in the signature with no word that it is normally left out; say so there and at the connect step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
Codegen evals (CoCo and generic alike) lost 1-5 of 38 notebook-only cases to the 'add a destination later' example written as a python code block, which is a code cell that writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
…tionOutputConfig Generic Databricks notebooks put binary_classification_threshold on PredictionOutputConfig in 6 of 42 cases (S3 2 of 112; files and CoCo 0), a TypeError at predict time. The reference gave the signature but never said the config takes neither argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
Eval results: generic host against CoCoHarness: Codegen, first-try match against the reference specs (38 confirm cases)
End to end on a GPU: train, predict, write, read backThe notebooks that matched their spec, from the latest full sweeps. All runs install
These sweeps predate the last fixes. Of the first-try failures, these are fixed on this PR: Intake (90 core cases, preflight results injected; build
|
| Arm | Label | Rubric |
|---|---|---|
| CoCo, Snowflake | 80/90 | 43/90 |
| generic, Snowflake | 79/90 | 43/90 |
| generic, S3 | 80/90 | 47/90 |
generic, access check fails (noaccess, 47 complete requests) |
47/47 | 42/47 |
On the same Snowflake data, CoCo and generic disagree on 22 cases, split 11 each way: noise, not a
direction. The new access check (preflight §0) asks for access or for the structure 42/47 times;
the misses offered only the structure. Commits after 23802ec3 change only Step 2 files.
What each commit on this PR fixed, and where it was found
| Commit | Fix | Found by |
|---|---|---|
14a51ec2 |
bare file table names; write a path destination at exactly the confirmed path | codegen (files 2/38 → 36/38) |
8caea0e4, 5a150873, 76cd6f7f |
file population: create the directory, stage TIMESTAMP, cast it |
GPU runs |
7ed8896d |
Databricks population through the statement API, wait_timeout ≤ 50 s |
GPU runs |
feefa6da, ad5d7484 |
no region or profile placeholder for S3/GCS; region is optional |
SageMaker runs |
e6df3dbe |
the later-destination example as text, not a python block |
codegen, CoCo included (32/38 → 38/38) |
e35d691b |
threshold and top-k on predict, never on PredictionOutputConfig |
GPU runs (Databricks 34/38 → 38/38) |
Open
- A full GPU sweep on this PR's head, all three backends.
- Intake rerun on the head (expected unchanged).
- CoCo has no GPU baseline in this harness, so the end-to-end numbers are generic-only.
kumo_finetuning_sdk.md§7.1, the Snowflake population example, stagesENTITY1only. kumo-engine
1.0.0 rejects that for a timestamped entity, so CoCo likely fails that case too; it isn't measured
here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments: 2
agent-skills/skills/kumoai-reference/references/graph_construction.md: 2
76-77: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winQualify the connector-independent workflow claim.
The workflow is shared, but connector contracts are not. Table names and path forms differ, and file and object-store connectors do not support
sql(). State that the workflow is common while preserving these connector-specific limits.
149-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the documented CSV time-column contract.
For a CSV time column,
Dtype.datealone is not the documented fix. UseDtype.timewith an explicittimestamp_format. Otherwise generated file-connector graphs can misparse or lose their timeline.
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: NVIDIA/kumo-engine
- Branch: main
- File: .coderabbit.yaml
- Commit: 341cf04
- Layers: Repository YAML
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
5457b1c2-3028-4cac-a735-f3ee429a3d9d
📒 Files selected for processing: 21
agent-skills/DESIGN.mdagent-skills/README.mdagent-skills/scripts/package_skill.pyagent-skills/skills/hosts/generic/adapter.mdagent-skills/skills/hosts/generic/runtime.mdagent-skills/skills/hosts/snowflake-coco/adapter.mdagent-skills/skills/kumo-data-science/SKILL.mdagent-skills/skills/kumo-data-science/accept/batch_inference.mdagent-skills/skills/kumo-data-science/accept/intake_contract.mdagent-skills/skills/kumo-data-science/accept/preflight.mdagent-skills/skills/kumo-data-science/validate/examples/bad_request.mdagent-skills/skills/kumo-model-explanations/SKILL.mdagent-skills/skills/kumo-model-tuning/SKILL.mdagent-skills/skills/kumo-training-diagnostics/SKILL.mdagent-skills/skills/kumoai-reference/SKILL.mdagent-skills/skills/kumoai-reference/references/graph_construction.mdagent-skills/skills/kumoai-reference/references/kumo_finetuning_sdk.mdagent-skills/skills/kumoai-reference/references/outputs.mdagent-skills/skills/kumoai-reference/references/task_journeys.mdagent-skills/skills/kumoai-reference/references/troubleshooting_signals.mdagent-skills/tests/test_package_skill.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.
- Access: an S3/GCS listing proves access, not structure; keep looking for a column reader, else ask for the structure. - Path destinations end in '/', '.parquet' or '.csv'. File, S3 and GCS sinks all add '.parquet' to any other path (object_store_sink, connector.file), so the SDK reference's "FileConnector does not append" was wrong. Read back in the format the suffix names. - Population staging reads the entity source in its own format (CSV or Parquet) and still writes Parquet. - Binary threshold: 0.5 [default] at Node 1d unless the user gives one; predict requires it and has no default. - batch_inference: a connector named alone is a source only on a host that accepts connector names. - CoCo: a semantic view's connector scope is its base tables' schema. - graph_construction: connector-specific naming and sql(); non-ISO time strings take timestamp_format with Dtype.date (Dtype.time is time of day). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub Actions
Inactive
|
@coderabbitai On the two
|
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments: 2
agent-skills/skills/kumoai-reference/references/graph_construction.md: 1
152-155: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winResolve the timestamp contract before publishing this example.
The graph guidance recommends
Dtype.datefor CSV strings andtimestamp_format, but the SDK reference and engine README prescribeDtype.timefor CSV timestamps. The SDK reference also markstimestamp_formatas unverified. Verify SnowflakeTEXTand CSV behavior separately, then align both references or mark the behavior as unverified.agent-skills/skills/hosts/snowflake-coco/adapter.md: 1
97-98: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMap a connector-name scope to a connection identifier, not to
databaseorschema_name.Section 0 accepts "a connector name" as a
source_scopeform, but the closing paragraph of §6.1 says everysource_scopeis the connector'sdatabaseandschema_name. A connector-name-only scope therefore cannot supply the<DB>and<schema>for the §6.1 scope query or thedatabaseandschema_namein theruntime.md§3 connector. The Snowflake host text does not say how to resolve that name.runtime.md§3 already usesCONNECTION_NAMEas a separate connection identifier, so the name can serve that role. The database and schema must still come from the user.🐛 Suggested fix
-The `source_scope` the user confirmed at intake is also the `database` / `schema_name` passed to -the connector in `runtime.md` §3. For a semantic view, pass the database and schema its base +When the user confirmed a database and schema, pass them as the `database` / `schema_name` to +the connector in `runtime.md` §3. If the user confirmed only a connector name, use it only as the +`connection_name` in `runtime.md` §3 and ask for the database and schema. For a semantic view, pass the database and schema its base
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: NVIDIA/kumo-engine
- Branch: main
- File: .coderabbit.yaml
- Commit: 341cf04
- Layers: Repository YAML
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
d5e30866-3563-4abb-aadb-3bf18dbb5a4e
📒 Files selected for processing: 8
agent-skills/skills/hosts/generic/adapter.mdagent-skills/skills/hosts/generic/runtime.mdagent-skills/skills/hosts/snowflake-coco/adapter.mdagent-skills/skills/kumo-data-science/SKILL.mdagent-skills/skills/kumo-data-science/accept/batch_inference.mdagent-skills/skills/kumo-data-science/accept/intake_contract.mdagent-skills/skills/kumoai-reference/references/graph_construction.mdagent-skills/skills/kumoai-reference/references/kumo_finetuning_sdk.md
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
Each §3.1 recipe reads the entity table and writes the population in one system, but the rule picked the recipe by the destination, so a source and destination in different systems had no working branch. Stage in the source's system instead; Node 1d asks for a location there when the destination is elsewhere. CoCo is unchanged: its source and destination are both Snowflake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@Manushpm8 Follow-up on §3.1, fixed in bac10d9: the population is now staged in the source's system, whatever the destination. Each recipe (pyarrow for files/S3/GCS, the §7.1 SQL for Snowflake, the statement API for Databricks) already reads the entity table and writes the population inside one system, so choosing it by source makes every source/destination pair work without new code. Node 1d ( |
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
agent-skills/skills/kumo-data-science/SKILL.md: 1
222-225:Minor · Data Integrity & Integration · Quick win Resolve the population-staging location contradiction.
When the destination is in the source schema, Line 223 defaults to
<DESTINATION>_POPULATIONin that schema. Line 225 says never to stage in the source schema. State whether this location is allowed, then make the default and prohibition consistent.
-
-
- Source mode: Auto
- Repository: NVIDIA/kumo-engine
- Branch: main
- File: .coderabbit.yaml
- Commit: 341cf04
- Layers: Repository YAML
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
677c6eeb-9598-4e50-8c15-73bdd255c9c6
-
agent-skills/skills/hosts/generic/runtime.mdagent-skills/skills/kumo-data-science/SKILL.md
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
|
@Manushpm8 Everything from your review at b5d5245 is addressed. Could you take another look?
Two things to know: the §3.1 change hasn't been run end to end on a mixed source/destination pair yet, and a Databricks source reached through a cluster (no SQL warehouse) now reports population staging as not covered, like BigQuery. The connector pytest failure is still the BigQuery secrets, as you said. |
…y client runtime.md §3.1 said BigQuery population staging was not covered on the generic host. Stage it like Databricks: CREATE OR REPLACE TABLE ... AS SELECT with ENTITY1 and, when the entity has a time column, CAST(<time> AS TIMESTAMP) AS TIMESTAMP, waiting on the job, then a BigQueryConnector scoped to the population's dataset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Eval: generic host on GCS and BigQuery (skill
Data: the 4 eval schemas (31 tables) in
The intake label matches CoCo on both backends. Codegen has no hard fails; BigQuery has one soft diff ( GPU (SageMaker) not run yet. It is waiting for a GCP credential for the jobs; results will follow in another comment. |
Manushpm8
left a comment
Manushpm8
left a comment
There was a problem hiding this comment.
Re-reviewed at e1ce213. The three points from b5d5245 are fixed: TIMESTAMP is staged only when the entity table has a time column in the pyarrow, Databricks and BigQuery recipes; §3.1 stages in the source's system, so every source/destination pair has a working branch and Node 1d asks for a location there when the destination is elsewhere; kumo_finetuning_sdk.md:290 says Dtype.date. agent-skills/tests 162 pass and package_skill.py --check is clean for both hosts. The only red check is the two BigQuery permission tests missing secrets, same as main. One non-blocking nit inline on the population connector.
FileConnector rejects a URI, so an S3 or GCS source failed after staging. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One conflict in accept/batch_inference.md's source_scope bullet, resolved by keeping #48's connector-name qualifier and this branch's "a source not in the conversation is not missing". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kumo-engine 1.0.0's BigQuery source backend raises when a connector names no identity, though the connector's docstring says None defers to ambient credentials. BigQueryIdentity() with no arguments is the ambient Google credential, so naming it changes nothing else. Found by the generic-host GPU eval on SageMaker (GCS with no identity passed; BigQuery failed at table_names()). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…med a billing project The GPU eval found 5/38 BigQuery notebooks stopping at a '<BIGQUERY_BILLING_PROJECT>' placeholder no user asked for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oject, never a placeholder
The GPU eval found 7/38 notebooks leaving PROJECT_ID as a fill-in
('the GCP project billed for queries') although the conversation confirmed
kumo-engineering.<dataset>, and 9/38 adding a billing placeholder.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Manushpm8
left a comment
Manushpm8
left a comment
There was a problem hiding this comment.
Re-approve at 1178a6d. The changes since e1ce213 (population connector picked from the path, BigQuery identity and project scope, the populations rules) match the SDK, and all my threads are resolved. One non-blocking nit inline in the SDK reference. Also: the SKILL.md description is 1188 characters and the Agent Skills spec caps description at 1024; #111 grows it further, so trim it in whichever lands first.
| | `SnowflakeConnector` | `artifact_type=` + `table_name=` (bare name) | connector scoped to the destination database and schema; §9.1 | | ||
| | `FileConnector`, `S3Connector`, `GCSConnector` | `output_paths={'predictions': ..., 'embeddings': ...}` — the keys select the artifacts — or `artifact_type=` + `table_name=` | do not combine the two forms; a path ending in `/` is a directory and must be empty; otherwise one file, written in the format its `.parquet` or `.csv` suffix names, and `.parquet` is added when it has neither — by all three connectors; paths are relative to a rooted connector, full URIs for a rootless one | | ||
| | `BigQueryConnector` | `artifact_type=` + `table_name=` (unqualified table ID in its dataset) | predictions and embeddings only — not a model `export_config` and not a `Trainer.load_from_connector` source | | ||
| | `DatabricksConnector` | `artifact_type=` + `table_name='<schema>__kumo__<table>'` | the schema must be the connector's own; a bare table name fails; writes stage through the volume `<catalog>.<schema>._kumo_staging`, created on first use | |
There was a problem hiding this comment.
Non-blocking: "a bare table name fails" is not right. kumo_databricks/output.py within_schema prefixes a bare table_name with the connector's schema_name, so table_name='<table>' works and lands as <schema>__kumo__<table>. Keep the row, say the prefix is applied for you. The same sentence is in #111's hosts/databricks-genie/runtime.md §4.
Generic host on GCS and BigQuery: GPU results (SageMaker)Each notebook ran as a SageMaker training job (T4, A10G, L4), with up to 3 repair turns. Google auth goes through Workload Identity Federation from the SageMaker role to
In each backend, 1 case is a harness error: a dependency download was cut off at job start, so the notebook never ran. Intake is unchanged ( Skill changes from this run, all on #48:
SDK bug, uc30 on BigQuery (10 cases): the BigQuery source samples with Details: |
§1 exempted S3 and GCS from runtime-only constants but not BigQuery, so 6/38 BigQuery notebooks added a '<BILLING_PROJECT_ID>' placeholder and stopped at the fill-in guard. BigQuery joins the exemption: it bills to the confirmed dataset's project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Generic host, BigQuery GPU re-run on skill
Details: |
Summary
The
kumo-data-scienceskill shipped only to Snowflake CoCo, and its "portable" core assumed Snowflake throughout. This adds agenerichost for coding agents working beside the user (Claude Code, Codex). The host generates code for any of the sixkumo.engineconnectors: File, S3, GCS, BigQuery, Databricks and Snowflake. CoCo behavior is unchanged.Access-aware preflight (new §0 in
accept/preflight.md)source_scopewith a read-only tool it already has. That can be a tool in the session, an authenticated CLI (aws,gcloud,bq,databricks,snow,duckdb) run against the scope itself, or Python withkumo.engine.[inferred, unverified].hosts/generic/adapter.mdcovers source forms mapped to connectors, the access checks per system, destinations, and how the capability skills apply off Snowflake.runtime.mdcovers:Core changes
hosts/<host>/. Source and destination forms are now defined by the host; batch inference no longer rejectss3://and file destinations by itself.kumo_finetuning_sdk.mdgains §3.4 (non-Snowflake connectors) and §9.2 (destinations by connector), both read from the package source.package_skill.py genericbuilds a 29-file package. New tests check that core files name no host, that each package ships only its own host, and that every adapter covers source scope, access and destinations.Test plan
pytest agent-skills/tests: 161 passed (148 before).package_skill.py --check: coco 30 files and generic 29 files, 0 unresolved references.bqabsent). It checked its session tools,bqand Python, and stopped before the graph proposal with the access-or-details ask-back. It restated the four task fields, wrote no code, and quoted the correct build marker.runtime.mdsays so and tells the agent to tell the user. The next real test isFileConnectoron Parquet exports of the eval schemas.Known gaps
🤖 Generated with Claude Code