← back to Cli Printing Press

fix(generator): gate table creation on hasDomainUpsert (#465 follow-up) (#467)

05092ed1ed8fbbadec5193dcaba1d71d841a5e55 · 2026-05-01 14:46:12 -0700 · Trevin Chow

Brings table creation in line with typed-Upsert generation. Both are
now conditional on the same gate:

  (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)

Previously tables were created unconditionally while typed Upserts
were gated. The asymmetry produced dead tables for resources whose
names collided with framework cobra commands (auth, search, version):

  - Resource "auth" gets renamed `<api-slug>-auth` (e.g., `cal-com-auth`)
  - The renamed table only has id/data/synced_at columns (typed-column
    extraction runs against the original name and finds nothing)
  - The >3-column gate filters out the typed Upsert
  - UpsertBatch dispatches by resource type but has no case for the
    renamed type, so inserts go only to the generic `resources` table
  - The typed table is created but never written to

Surfaced by sweep work on cal-com, pokeapi, coingecko, docker-hub —
each had pub-side `Upsert<X>` and `case "<x>":` switches that fresh
template stopped emitting because of this gate, leaving 3-col tables
that nothing populated. After this fix, the dead tables just don't
exist; data still flows via UpsertBatch + upsertGenericResourceTx.

Test added: TestStoreSkipsDeadTablesForResourcesWithoutTypedUpsert
generates a spec with both a typed-column resource and a
collision-renamed bare resource, asserts the renamed dead table is
not created and the typed-column resource keeps its table.

Verification:
- go test ./... — 2621 pass (1 new)
- go vet, golangci-lint clean
- Golden verify passed
- Regenerated cal-com: 6 dead tables removed (api_keys, cal_com_auth,
  cal_com_auth_2, conferencing, destination_calendars, organizations)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Files touched

• M internal/generator/generator.go
• M internal/generator/generator_test.go
• M internal/generator/templates/store.go.tmpl
• M internal/generator/templates/store_upsert_batch_test.go.tmpl
• M internal/generator/templates/sync.go.tmpl

Diff

commit 05092ed1ed8fbbadec5193dcaba1d71d841a5e55
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 1 14:46:12 2026 -0700

    fix(generator): gate table creation on hasDomainUpsert (#465 follow-up) (#467)
    
    Brings table creation in line with typed-Upsert generation. Both are
    now conditional on the same gate:
    
      (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)
    
    Previously tables were created unconditionally while typed Upserts
    were gated. The asymmetry produced dead tables for resources whose
    names collided with framework cobra commands (auth, search, version):
    
      - Resource "auth" gets renamed `<api-slug>-auth` (e.g., `cal-com-auth`)
      - The renamed table only has id/data/synced_at columns (typed-column
        extraction runs against the original name and finds nothing)
      - The >3-column gate filters out the typed Upsert
      - UpsertBatch dispatches by resource type but has no case for the
        renamed type, so inserts go only to the generic `resources` table
      - The typed table is created but never written to
    
    Surfaced by sweep work on cal-com, pokeapi, coingecko, docker-hub —
    each had pub-side `Upsert<X>` and `case "<x>":` switches that fresh
    template stopped emitting because of this gate, leaving 3-col tables
    that nothing populated. After this fix, the dead tables just don't
    exist; data still flows via UpsertBatch + upsertGenericResourceTx.
    
    Test added: TestStoreSkipsDeadTablesForResourcesWithoutTypedUpsert
    generates a spec with both a typed-column resource and a
    collision-renamed bare resource, asserts the renamed dead table is
    not created and the typed-column resource keeps its table.
    
    Verification:
    - go test ./... — 2621 pass (1 new)
    - go vet, golangci-lint clean
    - Golden verify passed
    - Regenerated cal-com: 6 dead tables removed (api_keys, cal_com_auth,
      cal_com_auth_2, conferencing, destination_calendars, organizations)
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/generator/generator.go                    |  9 +++
 internal/generator/generator_test.go               | 73 ++++++++++++++++++++++
 internal/generator/templates/store.go.tmpl         |  9 ++-
 .../templates/store_upsert_batch_test.go.tmpl      |  4 +-
 internal/generator/templates/sync.go.tmpl          |  2 +-
 5 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 24966d34..7789ebe5 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -297,6 +297,15 @@ func New(s *spec.APISpec, outputDir string) *Generator {
 		"hasDomainUpsert": func(name string) bool {
 			return domainUpsertMethodName(name) != "UpsertBatch"
 		},
+		// hasTypedTable is the single source of truth for "this table gets a
+		// typed Upsert<X>." Table creation, typed-Upsert generation, the
+		// UpsertBatch dispatch switch, and the populated-table tests must all
+		// gate on the same predicate; otherwise dead tables (created but never
+		// written to) leak in for resources whose names hit the framework-cobra
+		// rename and end up with only id/data/synced_at columns.
+		"hasTypedTable": func(t TableDef) bool {
+			return len(t.Columns) > 3 && t.Name != "sync_state" && domainUpsertMethodName(t.Name) != "UpsertBatch"
+		},
 		"pathContainsParam": func(path, name string) bool {
 			return strings.Contains(path, "{"+name+"}")
 		},
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index e49575d2..4e99d85d 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -5504,3 +5504,76 @@ func TestSearchTemplateEmitsEmptyJSONEnvelope(t *testing.T) {
 	assert.Less(t, jsonModeIdx, noResultsIdx,
 		"jsonMode check must come before the human-mode 'No results' stderr line; otherwise the JSON-envelope path is skipped on empty results")
 }
+
+// TestStoreSkipsDeadTablesForResourcesWithoutTypedUpsert pins the gate that
+// ties table creation to typed-Upsert generation. Without it, resources
+// whose names collide with framework cobra commands get renamed
+// `<api-slug>-<name>` and produce 3-column tables that nothing writes to —
+// UpsertBatch's switch has no case for them, so inserts only land in
+// `resources`. The dead typed table bloats schema without ever being
+// populated.
+func TestStoreSkipsDeadTablesForResourcesWithoutTypedUpsert(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := &spec.APISpec{
+		Name:    "demo",
+		Version: "0.1.0",
+		BaseURL: "https://api.example.com",
+		Auth:    spec.AuthConfig{Type: "none"},
+		Config:  spec.ConfigSpec{Format: "toml", Path: "~/.config/demo-pp-cli/config.toml"},
+		Resources: map[string]spec.Resource{
+			"auth": { // Will rename to `demo-auth` (framework collision)
+				Description: "Auth tokens",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/auth", Description: "List auth tokens"},
+				},
+			},
+			"items": { // Has typed columns -> keeps its table + Upsert
+				Description: "Items",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method:      "GET",
+						Path:        "/items",
+						Description: "List items",
+						Params: []spec.Param{
+							{Name: "id", Type: "string"},
+							{Name: "name", Type: "string"},
+							{Name: "status", Type: "string"},
+							{Name: "created_at", Type: "string", Format: "date-time"},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	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)
+	store := string(storeSrc)
+
+	// Pin the exact CREATE TABLE set: only resources, sync_state (always
+	// emitted), and items (has typed columns) survive. No demo_auth, no
+	// renamed-with-suffix dead table, nothing else snuck in.
+	createRe := regexp.MustCompile("`CREATE TABLE IF NOT EXISTS (\\w+) \\(")
+	gotTables := map[string]bool{}
+	for _, m := range createRe.FindAllStringSubmatch(store, -1) {
+		gotTables[m[1]] = true
+	}
+	assert.Equal(t, map[string]bool{"resources": true, "sync_state": true, "items": true}, gotTables,
+		"only typed-table resources + the always-on resources/sync_state should be created")
+
+	// items keeps its typed Upsert; demo_auth/demo-auth gets none.
+	assert.Contains(t, store, "func (s *Store) UpsertItems(", "typed Upsert should exist for items")
+	assert.NotRegexp(t, regexp.MustCompile(`func \(s \*Store\) Upsert(Demo)?Auth\(`), store,
+		"no typed Upsert for the renamed dead resource (under any casing)")
+
+	// Renamed resources still flow through the generic path. Anchor the
+	// claim that UpsertBatch keeps writing to `resources` for unknown types.
+	assert.Contains(t, store, "upsertGenericResourceTx(tx, resourceType, id",
+		"UpsertBatch must still call upsertGenericResourceTx so renamed resources land in `resources`")
+}
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index 5feffd69..cd8dcd13 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -241,7 +241,10 @@ func (s *Store) migrate(ctx context.Context) error {
 			id, resource_type, content, tokenize='porter unicode61'
 		)`,
 {{- range .Tables}}
-{{- if ne .Name "sync_state"}}
+{{- /* Gate: only create tables that have a typed Upsert. Otherwise the
+       table is dead — UpsertBatch's dispatch skips it and inserts go only
+       to the generic `resources` table. */}}
+{{- if hasTypedTable .}}
 		`CREATE TABLE IF NOT EXISTS {{safeName .Name}} (
 {{- range $i, $col := .Columns}}
 {{- if $i}},{{end}}
@@ -603,7 +606,7 @@ func lookupFieldValue(obj map[string]any, snakeKey string) any {
 }
 
 {{- range .Tables}}
-{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- if hasTypedTable .}}
 // 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.
@@ -769,7 +772,7 @@ func (s *Store) UpsertBatch(resourceType string, items []json.RawMessage) (int,
 
 		switch resourceType {
 {{- range .Tables}}
-{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- if hasTypedTable .}}
 		case "{{.Name}}":
 			if err := s.upsert{{pascal .Name}}Tx(tx, id, obj, item); err != nil {
 				return 0, extractFailures, fmt.Errorf("typed upsert for %s/%s: %w", resourceType, id, err)
diff --git a/internal/generator/templates/store_upsert_batch_test.go.tmpl b/internal/generator/templates/store_upsert_batch_test.go.tmpl
index 16cd2115..18495154 100644
--- a/internal/generator/templates/store_upsert_batch_test.go.tmpl
+++ b/internal/generator/templates/store_upsert_batch_test.go.tmpl
@@ -260,13 +260,13 @@ func TestUpsertBatch_ExtractFailuresReturnedForPerItemMisses(t *testing.T) {
 
 {{- $hasTyped := false }}
 {{- range .Tables}}
-{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- if hasTypedTable .}}
 {{- $hasTyped = true }}
 {{- end}}
 {{- end}}
 {{- if $hasTyped }}
 {{- range .Tables}}
-{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- if hasTypedTable .}}
 {{- $hasParentID := false }}
 {{- range .Columns}}{{if eq .Name "parent_id"}}{{$hasParentID = true}}{{end}}{{end}}
 
diff --git a/internal/generator/templates/sync.go.tmpl b/internal/generator/templates/sync.go.tmpl
index 77849161..baeac779 100644
--- a/internal/generator/templates/sync.go.tmpl
+++ b/internal/generator/templates/sync.go.tmpl
@@ -700,7 +700,7 @@ func upsertSingleObject(db *store.Store, resource string, data json.RawMessage)
 
 	switch resource {
 {{- range .Tables}}
-{{- if and (gt (len .Columns) 3) (ne .Name "sync_state") (hasDomainUpsert .Name)}}
+{{- if hasTypedTable .}}
 	case "{{.Name}}":
 		return db.Upsert{{pascal .Name}}(data)
 {{- end}}

← 45a7722b fix(openapi): preserve params on single-endpoint specs (#465  ·  back to Cli Printing Press  ·  chore(main): release 3.2.1 (#466) 08b88654 →