docs: describe attach as joining across instant databases - #309
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 2. [ ] `hotdata search list` — avoid duplicate text/vector indexes on the same column | ||
| 3. [ ] Create index (address by name): | ||
| - [ ] **Instant DB only:** `hotdata search create <tbl>_<col> --type text --from <alias>.public.<tbl> --column <text_col>` (vector: `--type vector [--provider <p>]`). An external catalog must be attached to an instant database first (`hotdata databases attach`). | ||
| - [ ] **Instant DB only:** `hotdata search create <tbl>_<col> --type text --from <alias>.public.<tbl> --column <text_col>` (vector: `--type vector [--provider <p>]`). A table in another database must be attached first (`hotdata databases attach <database>`). |
There was a problem hiding this comment.
This line keeps the claim the PR removes elsewhere. Replace it with the INDEXES.md rule: build the index in the database that owns the table, or load a copy into this one.
skills/hotdata/subskills/search/references/INDEXES.md:42 now states that attaching does not make a table indexable. This checklist step still tells the agent to attach first, and it links to INDEXES.md as its detail page.
Failure scenario: an agent follows the Retrieval checklist, runs hotdata databases attach other-db --alias o, then hotdata search create t_body --type text --from o.public.t --column body. The server rejects the create, because the target table belongs to a read-only attached database. The agent has no next step, because the checklist named attach as the fix.
| JOIN prod.public.orders o ON o.ticket_id = t.id" | ||
| ``` | ||
|
|
||
| The attached database is read-only here: loads still go to your own database, |
There was a problem hiding this comment.
nit: the command table at README.md:168 still reads "Attach a catalog so its tables are queryable", and line 169 still reads "Detach a previously attached catalog" (not blocking). A reader who scans the table sees the old model, two sections below the new one. Reword both rows to match this section.
| # Attach a catalog so its tables are queryable (enables cross-catalog queries — see below) | ||
| hotdata databases attach <catalog|name> [--database <id>] [--alias <alias>] | ||
| hotdata databases detach <catalog|name|alias> [--database <id>] | ||
| # Attach another database so its tables are queryable (enables cross-database queries — see below) |
There was a problem hiding this comment.
super nit: hotdata databases attach --help still describes the old model (not blocking). src/commands/databases.rs:128 reads "Attach a catalog to an instant database", and the --attach help at src/commands/databases.rs:69 uses --attach github as its example. The skill docs and the CLI help now disagree. Updating those doc comments needs the matching assertions in tests/databases_cli.rs checked.
There was a problem hiding this comment.
Review
Blocking Issues
skills/hotdata/references/WORKFLOWS.md:66— the Retrieval checklist still says a table in another database must be attached before indexing.skills/hotdata/subskills/search/references/INDEXES.md:42in this same PR says attaching does not make a table indexable. The two documents contradict each other, and WORKFLOWS.md links to INDEXES.md as its detail page.
Action Required
- Rewrite
WORKFLOWS.md:66to match the INDEXES.md rule: build the index in the database that owns the table, or load a copy into the current database.
Two non-blocking comments are inline: the README command table and the clap help strings still use the old catalog wording.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
WORKFLOWS.md told an agent to attach another database before indexing its table, which INDEXES.md in the same change says does not work — an attached database is read-only, so the index create is rejected and the checklist offers no next step. Both now say to build the index in the database that owns the table. Also brings the README command table and the clap help for attach and create --attach onto the new wording, with the assertion that pins the documented alias form.
| /// Read-only: loads still target this database's own catalog. Not | ||
| /// transitive: you see what you attached, not what it attached. | ||
| Attach { | ||
| /// Catalog name or id to attach (e.g. `github`) |
There was a problem hiding this comment.
super nit: this argument help keeps the catalog model that the long help above now replaces (not blocking).
A reader of hotdata databases attach --help sees both models on one screen. The github example is the workspace-catalog case this PR removes from README.md and skills/hotdata/SKILL.md.
src/commands/databases.rs:147 and src/commands/databases.rs:152 carry the same old wording. Reword all three. No test asserts on them — tests/databases_cli.rs:54 checks only --alias and --database.
…atalog The positional arg, --alias, Detach, the show label, the attach success message and one line of SKILL.md still described attach as taking a catalog while everything around them had moved to 'another instant database'. The positional now renders as <DATABASE>, matching the documented synopsis, and --alias says when it is required: the attached database's own catalog alias is the default, and 'default' cannot be attached under its own name, so a database created without --catalog needs one.
|
Second pass from the full review: the remaining old-vocabulary strings are gone — positional arg (now renders Also documented, because runtimedb#1412's review surfaced it: Two review candidates I did not act on, deliberately:
|
There was a problem hiding this comment.
All four prior threads are resolved: the WORKFLOWS.md retrieval step now states the ownership rule, the README command table rows match the new section, and the clap doc comments plus positional value names no longer describe the catalog model. Docs and --help agree. No remaining findings.
Note: the CI / test check was still queued when this review ran, so the updated assertions in tests/databases_cli.rs are not confirmed passing here.
1 participant
Summary
Docs half of runtimedb#1382 / runtimedb#1412, which makes
attachtake an instant database's catalog — the only kind of catalog the product still creates. The CLI needs no code change for that (its resolver already turns a database name intodefault_connection_id), but the docs still describe attach as a workspace-catalog operation, and one of them documents a command that could never have worked.The README's "Join across sources" example attaches
prod-replica— a thing a reader would reasonably hold as an instant database, and which 409'd until now. It now reads as what it always looked like.Changes throughout: "attach a catalog" → "attach another instant database"; "cross-catalog join" → "cross-database join"; the stale phrases "workspace catalog" and "external catalog" are gone, along with the anchors that pointed at the old section name.
Three facts that were not documented anywhere and are worth an agent knowing:
skills/hotdata/subskills/search/**claimed you could index a table by attaching its catalog first; you cannot, and it now says to index it in its own database instead.expires_at. That asymmetry is the one thing likely to surprise someone, so it is called out where attach is documented.Files:
README.md,skills/hotdata/SKILL.md,references/WORKFLOWS.md,references/DATA_MODEL.template.md,references/MODEL_BUILD.md,subskills/analytics/SKILL.md,subskills/search/SKILL.md,subskills/search/references/INDEXES.md.Test plan
cargo test— all targets pass (docs-only change; the clap help assertions intests/databases_cli.rsstill hold).#querying-across-catalogs-attachanchor, andrg -i "cross-catalog|workspace catalog|external catalog"overskills/andREADME.mdis empty.Merge after runtimedb#1412 deploys — until then the README example still 409s.