fix(graph): persist + read node.Context so cat context works on a mounted .db (mache-b8fe72)#467
Conversation
…t` works on a mounted .db The headline `context` virtual file was absent for every construct on a mounted/built .db: node.Context is populated at ingest (engine_walk.go) and works in MemoryStore, but the SQLite path dropped it — the nodes table had no context column, SQLiteWriter.AddNode never wrote it, and neither GetNode read path (SQLiteWriter.GetNode, NodesTableReader.GetNode) selected it. So vfs.ContextHandler always saw len(node.Context)==0 and served nothing. Fix, end to end: - nodes table gains a `context BLOB` column; AddNode persists n.Context. - Both GetNode paths select it. SQLiteWriter.GetNode must round-trip it too or the engine two-pass write nulls it via INSERT OR REPLACE (same class as the Properties fix, mache-d28eb1). - NodesTableReader (the mount read path, shared by SQLiteGraph + WritableGraph) selects context only when the column exists (new graph.ColumnExists), so old / leyline-produced nodes tables degrade to empty Context instead of erroring. Incremental builds ALTER the column in. Tests: writer persistence + writer GetNode round-trip (ingest), mount-path NodesTableReader.GetNode carries Context + backward-compat missing-column (graph). Closes the claim-vs-reality gap on the blog M2 feature.
find_smells (advisory)Scoped to files changed in this PR. Rules below run on the standalone (no-LLO) cross-ref tables; untested_function — 1 finding(s) in changed files
god_file — 1 finding(s) in changed files
Rules: the registry is |
There was a problem hiding this comment.
Pull request overview
This PR fixes a missing persistence/read-path for node.Context in the SQLite-backed graph so the context virtual file (used by cat context) works correctly when operating on a mounted or built .db.
Changes:
- Add a
context BLOBcolumn to the SQLitenodestable and persistnode.Contextduring ingestion writes. - Ensure both SQLite read paths (
SQLiteWriter.GetNodeandNodesTableReader.GetNode) round-tripContext, including guarding the mount reader for older DBs missing the column. - Add targeted tests covering write persistence, read round-trip, and backward-compat behavior when the column is absent.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/ingest/sqlite_writer.go | Adds nodes.context schema + incremental ALTER, persists and reads Context in writer paths. |
| internal/ingest/sqlite_writer_test.go | Adds an ingest-side regression test ensuring Context is stored and round-trips via GetNode. |
| internal/graph/nodes_table_reader.go | Adds column detection and conditionally selects context for backward-compatible mount reads. |
| internal/graph/context_roundtrip_test.go | Adds graph-layer tests for mount-path context read and missing-column compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The bug (first-impression / headline feature)
mache … --inferprojects 236sourcefiles but 0contextfiles — the blog's headline differentiatorcat context("only the imports/types visible to this scope") is absent for every construct on a mounted or built.db.Root cause (confirmed by tracing the data flow):
node.Contextis populated at ingest (engine_walk.go) and works in MemoryStore, but the SQLite path silently drops it:nodestable had nocontextcolumn,SQLiteWriter.AddNodenever wrote it,SQLiteWriter.GetNode,NodesTableReader.GetNode) selected it.So
vfs.ContextHandleralways sawlen(node.Context)==0and served nothing. Same claim-vs-reality class as the earlierfind_definition-broken-on-projection finding.Fix (end to end)
nodesgains acontext BLOBcolumn;AddNodepersistsn.Context(nil → NULL).GetNodepaths select it.SQLiteWriter.GetNodemust round-trip Context too, or the engine's two-pass write nulls it viaINSERT OR REPLACE— exactly the Properties bugmache-d28eb1.NodesTableReader(the mount read path shared bySQLiteGraph+WritableGraph) selectscontextonly when the column exists (newgraph.ColumnExists), so old / leyline-produced nodes tables degrade to empty Context instead of erroring. Incremental buildsALTERthe column in.Tests (the gap was: no test covered the SQLite path)
internal/ingest: writer persists to thecontextcolumn (raw-SQL assert) +SQLiteWriter.GetNoderound-trips it (two-pass protection).internal/graph:NodesTableReader.GetNodecarries Context (mount path) + backward-compat when the column is missing.vfs.ContextHandlerwas already tested againstnode.Context; the chain is now covered ingest → persist → read → serve.Verification
go test ./internal/graph/ ./internal/ingest/ ./internal/vfs/→ ok-raceon the round-trip/reader → okgo vet+golangci-lint(pre-commit) → pass