← back to Cli Printing Press
fix(cli): dispatch UpsertBatch to typed tables (#268) (#271)
8eea3a0e4bb02e380458cfc2062ea5a04afa31ca · 2026-04-24 21:09:40 -0700 · Trevin Chow
UpsertBatch wrote only to the generic resources table, leaving every
typed domain table empty after paginated syncs. The typed-table dispatch
existed in upsertSingleObject but only fired for single-object responses
(len(items) == 0), so paginated arrays — i.e. virtually every list
endpoint — went straight past it. Dependent-resource sync injected
parent_id into each item's JSON but the typed parent_id column stayed
NULL because UpsertBatch never touched the typed table.
Split the typed Upsert<Pascal> generator into upsert<Pascal>Tx (typed
INSERT + standalone FTS only, takes an existing *sql.Tx) plus a thin
public wrapper. UpsertBatch now calls upsertGenericResourceTx for the
generic insert and dispatches to upsert<Pascal>Tx through a generated
switch — all in the same transaction. Side-effect cleanup: batch and
single-object paths now share ON CONFLICT DO UPDATE semantics and
identical FTS rowid handling.
Regression coverage:
- New generated test template store_upsert_batch_test.go.tmpl emits
TestUpsertBatch_Populates<Pascal>Table for every typed table and
TestUpsertBatch_Sets<Pascal>ParentID for tables with parent_id.
Every printed CLI now ships the regression test.
- New generator-level test asserts UpsertBatch contains the dispatch
for typed resources. Extended TestGenerateDependentSyncCompiles to
run the generated store tests.
Public API unchanged. File-count expectations bumped by 1 for the new
generated test.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Files touched
M internal/generator/generator.goM internal/generator/generator_test.goM internal/generator/templates/store.go.tmplA internal/generator/templates/store_upsert_batch_test.go.tmpl
Diff
commit 8eea3a0e4bb02e380458cfc2062ea5a04afa31ca
Author: Trevin Chow <trevin@trevinchow.com>
Date: Fri Apr 24 21:09:40 2026 -0700
fix(cli): dispatch UpsertBatch to typed tables (#268) (#271)
UpsertBatch wrote only to the generic resources table, leaving every
typed domain table empty after paginated syncs. The typed-table dispatch
existed in upsertSingleObject but only fired for single-object responses
(len(items) == 0), so paginated arrays — i.e. virtually every list
endpoint — went straight past it. Dependent-resource sync injected
parent_id into each item's JSON but the typed parent_id column stayed
NULL because UpsertBatch never touched the typed table.
Split the typed Upsert<Pascal> generator into upsert<Pascal>Tx (typed
INSERT + standalone FTS only, takes an existing *sql.Tx) plus a thin
public wrapper. UpsertBatch now calls upsertGenericResourceTx for the
generic insert and dispatches to upsert<Pascal>Tx through a generated
switch — all in the same transaction. Side-effect cleanup: batch and
single-object paths now share ON CONFLICT DO UPDATE semantics and
identical FTS rowid handling.
Regression coverage:
- New generated test template store_upsert_batch_test.go.tmpl emits
TestUpsertBatch_Populates<Pascal>Table for every typed table and
TestUpsertBatch_Sets<Pascal>ParentID for tables with parent_id.
Every printed CLI now ships the regression test.
- New generator-level test asserts UpsertBatch contains the dispatch
for typed resources. Extended TestGenerateDependentSyncCompiles to
run the generated store tests.
Public API unchanged. File-count expectations bumped by 1 for the new
generated test.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
internal/generator/generator.go | 3 +
internal/generator/generator_test.go | 75 ++++++++++++++-
internal/generator/templates/store.go.tmpl | 96 ++++++++++---------
.../templates/store_upsert_batch_test.go.tmpl | 103 +++++++++++++++++++++
4 files changed, 230 insertions(+), 47 deletions(-)
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index b7a40c0e..b44c33ab 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1315,6 +1315,9 @@ func (g *Generator) Generate() error {
if err := g.renderTemplate("store_schema_version_test.go.tmpl", filepath.Join("internal", "store", "schema_version_test.go"), storeData); err != nil {
return fmt.Errorf("rendering store schema version test: %w", err)
}
+ if err := g.renderTemplate("store_upsert_batch_test.go.tmpl", filepath.Join("internal", "store", "upsert_batch_test.go"), storeData); err != nil {
+ return fmt.Errorf("rendering store upsert batch test: %w", err)
+ }
}
// Render vision CLI commands
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 4e061454..3c09ee8c 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -36,9 +36,10 @@ func TestGenerateProjectsCompile(t *testing.T) {
// +1 for internal/cli/feedback.go (HeyGen-style in-band agent feedback channel)
// +1 for internal/store/schema_version_test.go (PRAGMA user_version gate, discrawl-inspired)
// +2 for internal/cli/which.go + which_test.go (capability-to-command resolver)
- {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 43},
- {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 48},
- {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 45},
+ // +1 for internal/store/upsert_batch_test.go (regression for issue #268: typed-table dispatch)
+ {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 44},
+ {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 49},
+ {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 46},
}
for _, tt := range tests {
@@ -926,6 +927,68 @@ func TestGenerateStoreWithBatchResourceDoesNotDuplicateUpsertBatch(t *testing.T)
runGoCommand(t, outputDir, "test", "./internal/store")
}
+// TestGenerateStoreUpsertBatchDispatchesToTypedTable is the regression test
+// for issue #268. UpsertBatch was writing only to the generic resources
+// table, leaving typed tables empty after every paginated sync. The fix
+// added a switch dispatch inside UpsertBatch's transaction. This test
+// generates a spec with a typed table, then runs the generated store
+// tests — the emitted TestUpsertBatch_Populates*Table tests fail if the
+// dispatch ever regresses.
+func TestGenerateStoreUpsertBatchDispatchesToTypedTable(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "ads",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/ads-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "campaigns": {
+ Description: "Manage campaigns",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/campaigns",
+ Description: "List campaigns",
+ Response: spec.ResponseDef{Type: "array"},
+ Params: []spec.Param{
+ {Name: "id", Type: "string"},
+ {Name: "name", Type: "string"},
+ {Name: "status", Type: "string"},
+ {Name: "account_id", Type: "string"},
+ },
+ },
+ },
+ },
+ },
+ }
+
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ gen := New(apiSpec, outputDir)
+ gen.VisionSet = VisionTemplateSet{Store: true}
+ require.NoError(t, gen.Generate())
+
+ storeSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "store", "store.go"))
+ require.NoError(t, err)
+ src := string(storeSrc)
+
+ // The generator must emit the typed-table helper plus a wrapping public
+ // Upsert<Pascal> for the campaigns resource.
+ assert.Contains(t, src, "func (s *Store) upsertCampaignsTx(", "typed Tx helper missing for campaigns")
+ assert.Contains(t, src, "func (s *Store) UpsertCampaigns(", "public typed upsert missing for campaigns")
+
+ // UpsertBatch must dispatch to the typed helper inside its switch.
+ assert.Regexp(t, `(?s)func \(s \*Store\) UpsertBatch\(.*case "campaigns":\s+if err := s\.upsertCampaignsTx\(`, src,
+ "UpsertBatch must dispatch to upsertCampaignsTx — without this, paginated syncs leave typed tables empty (issue #268)")
+
+ runGoCommand(t, outputDir, "mod", "tidy")
+ runGoCommand(t, outputDir, "test", "./internal/store")
+}
+
// --- Unit 7: Feature Verification Tests ---
func generatePetstore(t *testing.T) string {
@@ -2885,9 +2948,13 @@ func TestGenerateDependentSyncCompiles(t *testing.T) {
assert.Contains(t, syncContent, `"messages"`, "sync.go should reference messages as a dependent resource")
assert.Contains(t, syncContent, `"channels"`, "sync.go should reference channels as the parent")
- // The generated project should compile
+ // The generated project should compile and the generated store tests
+ // should pass — including TestUpsertBatch_SetsMessagesParentID, which
+ // verifies dependent-resource sync fills the typed parent_id column
+ // (issue #268).
runGoCommand(t, outputDir, "mod", "tidy")
runGoCommand(t, outputDir, "build", "./...")
+ runGoCommand(t, outputDir, "test", "./internal/store")
}
func TestGenerateGraphQLCompiles(t *testing.T) {
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index 5db059e2..0511b1c9 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -345,29 +345,13 @@ func lookupFieldValue(obj map[string]any, snakeKey string) any {
{{- range .Tables}}
{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
-// Upsert{{pascal .Name}} inserts or updates a {{.Name}} record with domain-specific columns.
-func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
- var obj map[string]any
- if err := json.Unmarshal(data, &obj); err != nil {
- return fmt.Errorf("unmarshaling {{.Name}}: %w", err)
- }
-
- id := extractObjectID(obj)
- if id == "" {
- return fmt.Errorf("missing id for {{.Name}}")
- }
-
- tx, err := s.db.Begin()
- if err != nil {
- return err
- }
- defer tx.Rollback()
-
- if err := s.upsertGenericResourceTx(tx, "{{.Name}}", id, data); err != nil {
- return err
- }
-
- _, err = tx.Exec(
+// upsert{{pascal .Name}}Tx writes the typed-table portion of a {{.Name}} upsert
+// inside an existing transaction. The caller is responsible for the generic
+// resources insert (via upsertGenericResourceTx) and for committing the tx.
+// Splitting this out lets UpsertBatch dispatch typed inserts per item without
+// opening a per-item transaction.
+func (s *Store) upsert{{pascal .Name}}Tx(tx *sql.Tx, id string, obj map[string]any, data json.RawMessage) error {
+ if _, err := tx.Exec(
`INSERT INTO {{safeName .Name}} ({{columnNames .Columns}})
VALUES ({{columnPlaceholders .Columns}})
ON CONFLICT(id) DO UPDATE SET {{updateSet .Columns}}`,
@@ -379,9 +363,8 @@ func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
lookupFieldValue(obj, "{{.Name}}"),
{{- end}}
{{- end}}
- )
- if err != nil {
- return err
+ ); err != nil {
+ return fmt.Errorf("insert into {{.Name}}: %w", err)
}
{{- if and .FTS5 (not .FTS5Triggers)}}
@@ -399,6 +382,34 @@ func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
)
{{- end}}
+ return nil
+}
+
+// Upsert{{pascal .Name}} inserts or updates a {{.Name}} record with domain-specific columns.
+func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
+ var obj map[string]any
+ if err := json.Unmarshal(data, &obj); err != nil {
+ return fmt.Errorf("unmarshaling {{.Name}}: %w", err)
+ }
+
+ id := extractObjectID(obj)
+ if id == "" {
+ return fmt.Errorf("missing id for {{.Name}}")
+ }
+
+ tx, err := s.db.Begin()
+ if err != nil {
+ return err
+ }
+ defer tx.Rollback()
+
+ if err := s.upsertGenericResourceTx(tx, "{{.Name}}", id, data); err != nil {
+ return err
+ }
+ if err := s.upsert{{pascal .Name}}Tx(tx, id, obj, data); err != nil {
+ return err
+ }
+
return tx.Commit()
}
@@ -407,6 +418,12 @@ func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
// UpsertBatch inserts or replaces multiple records in a single transaction.
// This is 10-100x faster than individual Upsert calls for bulk operations.
+//
+// For resource types that have a domain-specific typed table, the per-item
+// generic insert is followed by a dispatch to the matching upsert<Pascal>Tx
+// inside the same transaction. Without that dispatch, paginated syncs would
+// only populate the generic resources table — typed tables (and indexed
+// columns like parent_id added by dependent-resource sync) would stay empty.
func (s *Store) UpsertBatch(resourceType string, items []json.RawMessage) error {
tx, err := s.db.Begin()
if err != nil {
@@ -436,26 +453,19 @@ func (s *Store) UpsertBatch(resourceType string, items []json.RawMessage) error
continue
}
- _, err := tx.Exec(
- `INSERT OR REPLACE INTO resources (id, resource_type, data, synced_at, updated_at)
- VALUES (?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
- id, resourceType, string(item),
- )
- if err != nil {
+ if err := s.upsertGenericResourceTx(tx, resourceType, id, item); err != nil {
return fmt.Errorf("upserting %s/%s: %w", resourceType, id, err)
}
- // Update FTS index (non-fatal — matches upsertGenericResourceTx pattern).
- // Use explicit rowid for FTS5 compatibility with modernc.org/sqlite.
- ftsRowid := ftsRowID(id)
- if _, ftsErr := tx.Exec(`DELETE FROM resources_fts WHERE rowid = ?`, ftsRowid); ftsErr != nil {
- fmt.Fprintf(os.Stderr, "warning: FTS index cleanup failed for %s/%s: %v\n", resourceType, id, ftsErr)
- }
- if _, ftsErr := tx.Exec(
- `INSERT INTO resources_fts (rowid, id, resource_type, content) VALUES (?, ?, ?, ?)`,
- ftsRowid, id, resourceType, string(item),
- ); ftsErr != nil {
- fmt.Fprintf(os.Stderr, "warning: FTS index update failed for %s/%s: %v\n", resourceType, id, ftsErr)
+ switch resourceType {
+{{- range .Tables}}
+{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+ case "{{.Name}}":
+ if err := s.upsert{{pascal .Name}}Tx(tx, id, obj, item); err != nil {
+ return fmt.Errorf("typed upsert for %s/%s: %w", resourceType, id, err)
+ }
+{{- end}}
+{{- end}}
}
}
diff --git a/internal/generator/templates/store_upsert_batch_test.go.tmpl b/internal/generator/templates/store_upsert_batch_test.go.tmpl
new file mode 100644
index 00000000..a25feaf0
--- /dev/null
+++ b/internal/generator/templates/store_upsert_batch_test.go.tmpl
@@ -0,0 +1,103 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package store
+{{- $hasTyped := false }}
+{{- range .Tables}}
+{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- $hasTyped = true }}
+{{- end}}
+{{- end}}
+{{- if $hasTyped }}
+
+import (
+ "encoding/json"
+ "fmt"
+ "path/filepath"
+ "testing"
+
+ _ "modernc.org/sqlite"
+)
+{{- range .Tables}}
+{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- $hasParentID := false }}
+{{- range .Columns}}{{if eq .Name "parent_id"}}{{$hasParentID = true}}{{end}}{{end}}
+
+// TestUpsertBatch_Populates{{pascal .Name}}Table verifies that UpsertBatch
+// dispatches paginated items into both the generic resources table AND the
+// typed {{.Name}} table. Regression for issue #268: before the fix, paginated
+// syncs only filled the generic resources table, so domain commands that
+// query the typed table saw zero rows.
+func TestUpsertBatch_Populates{{pascal .Name}}Table(t *testing.T) {
+ dbPath := filepath.Join(t.TempDir(), "data.db")
+ s, err := Open(dbPath)
+ if err != nil {
+ t.Fatalf("open: %v", err)
+ }
+ defer s.Close()
+
+ items := []json.RawMessage{
+ json.RawMessage(`{"id": "test-001"}`),
+ json.RawMessage(`{"id": "test-002"}`),
+ json.RawMessage(`{"id": "test-003"}`),
+ }
+ if err := s.UpsertBatch("{{.Name}}", items); err != nil {
+ t.Fatalf("UpsertBatch: %v", err)
+ }
+
+ db := s.DB()
+
+ var generic int
+ if err := db.QueryRow(`SELECT COUNT(*) FROM resources WHERE resource_type = ?`, "{{.Name}}").Scan(&generic); err != nil {
+ t.Fatalf("count resources: %v", err)
+ }
+ if generic != len(items) {
+ t.Fatalf("resources count = %d, want %d", generic, len(items))
+ }
+
+ var typed int
+ typedQuery := fmt.Sprintf(`SELECT COUNT(*) FROM %s`, "{{safeName .Name}}")
+ if err := db.QueryRow(typedQuery).Scan(&typed); err != nil {
+ t.Fatalf("count {{.Name}}: %v", err)
+ }
+ if typed != len(items) {
+ t.Fatalf("{{.Name}} count = %d, want %d (typed table not populated by UpsertBatch)", typed, len(items))
+ }
+}
+{{- if $hasParentID }}
+
+// TestUpsertBatch_Sets{{pascal .Name}}ParentID verifies that dependent-resource
+// sync (which injects parent_id into each item's JSON) populates the typed
+// parent_id column when items go through UpsertBatch. Regression for issue #268.
+func TestUpsertBatch_Sets{{pascal .Name}}ParentID(t *testing.T) {
+ dbPath := filepath.Join(t.TempDir(), "data.db")
+ s, err := Open(dbPath)
+ if err != nil {
+ t.Fatalf("open: %v", err)
+ }
+ defer s.Close()
+
+ items := []json.RawMessage{
+ json.RawMessage(`{"id": "child-001", "parent_id": "parent-A"}`),
+ json.RawMessage(`{"id": "child-002", "parent_id": "parent-A"}`),
+ json.RawMessage(`{"id": "child-003", "parent_id": "parent-B"}`),
+ }
+ if err := s.UpsertBatch("{{.Name}}", items); err != nil {
+ t.Fatalf("UpsertBatch: %v", err)
+ }
+
+ db := s.DB()
+
+ var matchedA int
+ parentQuery := fmt.Sprintf(`SELECT COUNT(*) FROM %s WHERE parent_id = ?`, "{{safeName .Name}}")
+ if err := db.QueryRow(parentQuery, "parent-A").Scan(&matchedA); err != nil {
+ t.Fatalf("count by parent_id: %v", err)
+ }
+ if matchedA != 2 {
+ t.Fatalf("parent_id=parent-A count = %d, want 2 (parent_id column not populated)", matchedA)
+ }
+}
+{{- end}}
+{{- end}}
+{{- end}}
+{{- end}}
← 6a184771 fix(cli): stop doctor from claiming valid creds are invalid
·
back to Cli Printing Press
·
chore(main): release 2.3.2 (#269) 7c9cce48 →