← back to Cli Printing Press
fix(cli): Dub retro — FTS batch indexing, retry cap, dogfood auth, root dedup, dead code (#143)
349580afbfb388c6c3750f32c8403e599f180adb · 2026-04-06 10:43:16 -0700 · Trevin Chow
* fix(cli): fix FTS batch indexing, retry cap, root dedup, and dead code in templates
Five generator template fixes from the Dub retro (#142):
- UpsertBatch now populates resources_fts so search works after sync
- retryAfter capped at 60s to prevent hangs on malformed Retry-After headers
- Root.go.tmpl excludes VisionSet command names from Resources loop to prevent
duplicate AddCommand panics (e.g., API with /analytics endpoint)
- Removed dead formatCompact function from helpers.go.tmpl
- Added VisionTemplateSet.CmdNames() for the dedup exclusion set
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): dogfood auth check reads config.go alongside client.go
Bearer/Bot prefix strings may be constructed in config.go (via
AuthHeader()) rather than in client.go. The dogfood auth protocol
check now reads both files, fixing false "unknown" mismatches for
every bearer-auth CLI.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Files touched
A docs/plans/2026-04-06-001-fix-dub-retro-template-scorer-fixes-plan.mdA docs/retros/2026-04-06-dub-retro.mdM internal/generator/generator.goM internal/generator/templates/client.go.tmplM internal/generator/templates/helpers.go.tmplM internal/generator/templates/root.go.tmplM internal/generator/templates/store.go.tmplM internal/generator/vision_templates.goM internal/pipeline/dogfood.go
Diff
commit 349580afbfb388c6c3750f32c8403e599f180adb
Author: Trevin Chow <trevin@trevinchow.com>
Date: Mon Apr 6 10:43:16 2026 -0700
fix(cli): Dub retro — FTS batch indexing, retry cap, dogfood auth, root dedup, dead code (#143)
* fix(cli): fix FTS batch indexing, retry cap, root dedup, and dead code in templates
Five generator template fixes from the Dub retro (#142):
- UpsertBatch now populates resources_fts so search works after sync
- retryAfter capped at 60s to prevent hangs on malformed Retry-After headers
- Root.go.tmpl excludes VisionSet command names from Resources loop to prevent
duplicate AddCommand panics (e.g., API with /analytics endpoint)
- Removed dead formatCompact function from helpers.go.tmpl
- Added VisionTemplateSet.CmdNames() for the dedup exclusion set
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): dogfood auth check reads config.go alongside client.go
Bearer/Bot prefix strings may be constructed in config.go (via
AuthHeader()) rather than in client.go. The dogfood auth protocol
check now reads both files, fixing false "unknown" mismatches for
every bearer-auth CLI.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
...001-fix-dub-retro-template-scorer-fixes-plan.md | 247 +++++++++++++++++++++
docs/retros/2026-04-06-dub-retro.md | 173 +++++++++++++++
internal/generator/generator.go | 2 +
internal/generator/templates/client.go.tmpl | 11 +-
internal/generator/templates/helpers.go.tmpl | 14 --
internal/generator/templates/root.go.tmpl | 2 +-
internal/generator/templates/store.go.tmpl | 11 +
internal/generator/vision_templates.go | 26 +++
internal/pipeline/dogfood.go | 10 +-
9 files changed, 477 insertions(+), 19 deletions(-)
diff --git a/docs/plans/2026-04-06-001-fix-dub-retro-template-scorer-fixes-plan.md b/docs/plans/2026-04-06-001-fix-dub-retro-template-scorer-fixes-plan.md
new file mode 100644
index 00000000..ab204553
--- /dev/null
+++ b/docs/plans/2026-04-06-001-fix-dub-retro-template-scorer-fixes-plan.md
@@ -0,0 +1,247 @@
+---
+title: "fix: Dub retro — FTS indexing, retry cap, dogfood auth, dedup, dead code"
+type: fix
+status: active
+date: 2026-04-06
+origin: docs/retros/2026-04-06-dub-retro.md
+---
+
+# fix: Dub retro — FTS indexing, retry cap, dogfood auth, dedup, dead code
+
+## Overview
+
+Five generator template and scorer fixes surfaced during the Dub CLI generation retro (issue #142). The most critical: `UpsertBatch` doesn't populate FTS indexes, making search silently return empty results after sync — every generated CLI is affected. The remaining four are smaller fixes that each save manual intervention on every or most future generations.
+
+## Problem Frame
+
+During the Dub run, five systemic Printing Press issues required manual code edits that would recur on every future CLI:
+1. Search returns empty after sync (FTS not populated in batch path)
+2. CLI hangs indefinitely on rate-limited APIs (no retry cap)
+3. Dogfood falsely reports auth mismatch (checks wrong file)
+4. Duplicate command registration panics (resource name = vision name)
+5. Dead `formatCompact` function in every CLI (template emits unused code)
+
+## Requirements Trace
+
+- R1. After `sync --full` + `search <term>`, results must be returned (from retro F2, F3)
+- R2. Rate-limited responses never cause waits longer than 60 seconds (from retro F4)
+- R3. Dogfood correctly detects Bearer auth for all generated CLIs (from retro F5)
+- R4. No duplicate AddCommand panics when API has analytics/search/events endpoints (from retro F1)
+- R5. No dead functions emitted by default templates (from retro F6)
+
+## Scope Boundaries
+
+- Does not change the FTS schema, add new FTS tables, or change FTS trigger mode
+- Does not change retry count, backoff strategy, or adaptive rate limiter logic
+- Does not change scorecard auth scoring (only dogfood)
+- Does not change promoted command logic or how resources are classified
+- Does not add new template features — strictly bug fixes
+
+## Context & Research
+
+### Relevant Code and Patterns
+
+- `internal/generator/templates/store.go.tmpl`: `Upsert()` (line 165) calls `upsertGenericResourceTx` which handles both resources table + resources_fts. `UpsertBatch()` (line 332) only does `INSERT OR REPLACE INTO resources` — no FTS
+- `internal/generator/templates/sync.go.tmpl`: Line 237 calls `db.UpsertBatch()` for paginated sync. `upsertSingleObject()` (line 392) dispatches to per-table methods but is only used for non-array responses
+- `internal/generator/templates/client.go.tmpl`: `retryAfter()` at line 547 parses Retry-After header with no maximum bound
+- `internal/pipeline/dogfood.go`: `checkAuth()` at line 459 reads only `client.go`, but Bearer prefix is constructed in `config.go` via `AuthHeader()` method
+- `internal/generator/templates/root.go.tmpl`: Resources loop (line 100) and VisionSet.Analytics (line 122) can both emit `newAnalyticsCmd`
+- `internal/generator/templates/helpers.go.tmpl`: `formatCompact` at line 693 — no call sites in any template
+
+### Test Patterns
+
+- Tests use `t.TempDir()` with `writeTestFile()` helper
+- Testify: `require` for setup, `assert` for validation
+- `dogfood_test.go` creates mock CLI directory structures and runs `RunDogfood()`
+- `generator_test.go` runs full spec→generate→compile→verify cycles
+
+## Key Technical Decisions
+
+- **FTS fix in UpsertBatch, not sync**: Adding FTS indexing to `UpsertBatch` is simpler and lower-risk than changing sync to dispatch to per-table methods. The per-table FTS tables remain unpopulated (that's a separate, larger change) but `resources_fts` — which is what the search command uses — will be populated
+- **60-second cap**: Matches the fix applied during the Dub run. Generous enough for real rate limits, short enough to prevent hangs
+- **Dogfood: check both files**: Adding `config.go` to the auth check is the minimal fix. The alternative (moving Bearer string to client.go template) would be a workaround for a scorer bug
+- **Root dedup via VisionSet exclusion**: Adding VisionSet names to the Resources loop guard is the cleanest approach — it keeps the two registration paths independent
+
+## Implementation Units
+
+- [ ] **Unit 1: Add FTS indexing to UpsertBatch**
+
+**Goal:** `resources_fts` is populated during batch sync so search returns results
+
+**Requirements:** R1
+
+**Dependencies:** None
+
+**Files:**
+- Modify: `internal/generator/templates/store.go.tmpl`
+- Test: `internal/generator/generator_test.go`
+
+**Approach:**
+- In `UpsertBatch()` (line 332-362), add FTS `DELETE` + `INSERT` into `resources_fts` inside the transaction loop, after the `INSERT OR REPLACE INTO resources` statement
+- Follow the same pattern as `upsertGenericResourceTx` (lines 147-160): delete by id, then insert with (id, resourceType, jsonContent)
+- Use non-fatal error handling for FTS ops (match existing pattern: `fmt.Fprintf(os.Stderr, "warning: ...")`)
+
+**Patterns to follow:**
+- `upsertGenericResourceTx` in store.go.tmpl lines 147-160
+
+**Test scenarios:**
+- Happy path: Generate a CLI from testdata spec, verify `UpsertBatch` function body contains `resources_fts` INSERT statement
+- Happy path: Full generate→compile cycle still passes with the template change
+
+**Verification:**
+- Generated store.go contains FTS indexing in UpsertBatch
+- `go build ./...` passes on generated CLI
+- Search returns results after sync (verified via generated code inspection)
+
+---
+
+- [ ] **Unit 2: Cap retryAfter at 60 seconds**
+
+**Goal:** Rate-limited responses never cause waits longer than 60 seconds
+
+**Requirements:** R2
+
+**Dependencies:** None
+
+**Files:**
+- Modify: `internal/generator/templates/client.go.tmpl`
+- Test: `internal/generator/generator_test.go`
+
+**Approach:**
+- Add `const maxRetryWait = 60 * time.Second` before the function
+- After both integer parse and HTTP date parse, cap: `if d > maxRetryWait { return maxRetryWait }`
+
+**Patterns to follow:**
+- Existing `retryAfter` structure at client.go.tmpl line 547
+
+**Test scenarios:**
+- Happy path: Generated client.go contains `maxRetryWait` constant and cap logic
+- Happy path: Full generate→compile cycle passes
+
+**Verification:**
+- Generated client.go contains the 60-second cap
+- `go build ./...` passes
+
+---
+
+- [ ] **Unit 3: Fix dogfood Bearer detection to check config.go**
+
+**Goal:** Dogfood correctly detects Bearer auth when the string is in config.go
+
+**Requirements:** R3
+
+**Dependencies:** None
+
+**Files:**
+- Modify: `internal/pipeline/dogfood.go`
+- Test: `internal/pipeline/dogfood_test.go`
+
+**Approach:**
+- In `checkAuth()` (line 459), after reading `client.go`, also read `config.go` from `filepath.Join(dir, "internal", "config", "config.go")`
+- Combine both sources for the string search: `combinedSource := clientSource + configSource`
+- Search `combinedSource` for `"Bearer "` / `"Bot "` patterns
+
+**Patterns to follow:**
+- Existing `os.ReadFile` + `strings.Contains` pattern in dogfood.go lines 459-474
+
+**Test scenarios:**
+- Happy path: CLI with Bearer auth in config.go (not client.go) → dogfood reports MATCH
+- Happy path: CLI with Bearer auth in client.go → still reports MATCH (no regression)
+- Edge case: CLI with no auth in either file → reports "unknown" (no regression)
+- Edge case: CLI with Bot auth in config.go → reports MATCH for Bot
+
+**Verification:**
+- `go test ./internal/pipeline/ -run TestRunDogfood` passes
+- Existing dogfood tests continue to pass
+
+---
+
+- [ ] **Unit 4: Deduplicate VisionSet vs Resource AddCommand in root.go**
+
+**Goal:** No duplicate AddCommand when an API resource name matches a VisionSet command
+
+**Requirements:** R4
+
+**Dependencies:** None
+
+**Files:**
+- Modify: `internal/generator/templates/root.go.tmpl`
+- Test: `internal/generator/generator_test.go`
+
+**Approach:**
+- The collision names are the VisionSet command names: analytics, export, import, search, sync, tail
+- In the Resources loop (line 100-104), add a check against these names. Two options:
+ - Option A: Check VisionSet flags inline: `(not (and $.VisionSet.Analytics (eq $name "analytics")))`
+ - Option B: Build a combined exclusion set in generator.go before template execution
+- Option A is simpler and self-contained in the template. The guard at line 101 becomes:
+ ```
+ {{- if and (not (index $.PromotedResourceNames $name)) (ne $name "auth") (not (index $.VisionCmdNames $name))}}
+ ```
+ where `VisionCmdNames` is a `map[string]bool` passed from generator.go containing all enabled vision command names
+
+**Patterns to follow:**
+- `PromotedResourceNames` exclusion pattern at root.go.tmpl line 101
+- Generator data construction in generator.go
+
+**Test scenarios:**
+- Happy path: Generate from a spec with `/analytics` endpoint + VisionSet.Analytics=true → root.go has exactly one `newAnalyticsCmd` registration
+- Happy path: Generate from a spec without `/analytics` endpoint + VisionSet.Analytics=true → root.go has one `newAnalyticsCmd` from VisionSet
+- Edge case: Generate from a spec with `/search` endpoint + VisionSet.Search=true → no duplicate
+- Happy path: Full generate→compile cycle passes
+
+**Verification:**
+- `grep -c 'newAnalyticsCmd' generated/root.go` returns 1
+- `go build ./...` passes
+
+---
+
+- [ ] **Unit 5: Remove dead formatCompact from helpers template**
+
+**Goal:** No unused functions emitted by templates
+
+**Requirements:** R5
+
+**Dependencies:** None
+
+**Files:**
+- Modify: `internal/generator/templates/helpers.go.tmpl`
+- Test: `internal/generator/generator_test.go`
+
+**Approach:**
+- Remove the `formatCompact` function (lines 693-705) from helpers.go.tmpl entirely
+- Verify no template references it: `grep -r "formatCompact" internal/generator/templates/` should return nothing after removal
+
+**Patterns to follow:**
+- Other conditional helpers gated by `{{if}}` blocks in helpers.go.tmpl
+
+**Test scenarios:**
+- Happy path: Generated helpers.go does not contain `formatCompact`
+- Happy path: `go vet ./...` reports no dead code in generated CLI
+- Happy path: Full generate→compile cycle passes
+
+**Verification:**
+- `grep formatCompact generated/helpers.go` returns nothing
+- `go build ./...` passes
+
+## System-Wide Impact
+
+- **FTS fix (Units 1)**: Affects every generated CLI's search behavior. No API surface change — internal store method only
+- **Retry cap (Unit 2)**: Affects rate-limited API interactions. Lower-risk — only caps extreme values
+- **Dogfood fix (Unit 3)**: Affects dogfood scoring for all bearer-auth CLIs. May improve auth_protocol scores by 2-5 points
+- **Root dedup (Unit 4)**: Affects generated root.go for APIs with endpoint names matching vision commands. Narrow blast radius
+- **Dead code (Unit 5)**: Affects every generated CLI's helpers.go. Trivial removal
+
+## Risks & Dependencies
+
+| Risk | Mitigation |
+|------|------------|
+| FTS indexing in UpsertBatch slows sync | Non-fatal FTS ops + same pattern as existing Upsert. FTS inserts are fast within a transaction |
+| Dogfood change breaks existing tests | Run full `go test ./internal/pipeline/` before merging |
+| VisionCmdNames map not populated correctly | Build from the same VisionSet flags already used in root.go.tmpl |
+
+## Sources & References
+
+- **Origin document:** [docs/retros/2026-04-06-dub-retro.md](docs/retros/2026-04-06-dub-retro.md)
+- Related issue: #142
+- Template files: `internal/generator/templates/store.go.tmpl`, `client.go.tmpl`, `root.go.tmpl`, `helpers.go.tmpl`
+- Scorer: `internal/pipeline/dogfood.go`
diff --git a/docs/retros/2026-04-06-dub-retro.md b/docs/retros/2026-04-06-dub-retro.md
new file mode 100644
index 00000000..1c86f203
--- /dev/null
+++ b/docs/retros/2026-04-06-dub-retro.md
@@ -0,0 +1,173 @@
+# Printing Press Retro: Dub
+
+## Session Stats
+- API: Dub (link attribution platform)
+- Spec source: OpenAPI 3.0.3 from https://api.dub.co
+- Scorecard: 91/100 (Grade A) — post-polish; 96 pre-polish
+- Verify pass rate: 100%
+- Fix loops: 1 shipcheck + 1 polish
+- Manual code edits: 5 (duplicate registration, dead function, auth constant, analytics subcommand wiring, CLI description)
+- Features built from scratch: 8 (all transcendence features)
+
+## Findings
+
+### F1. Duplicate AddCommand for analytics in root.go (Bug)
+- **What happened:** root.go registered `newAnalyticsCmd` twice — once from the Resources loop (OpenAPI `/analytics` path) and once from VisionSet.Analytics. Runtime panic on startup.
+- **Scorer correct?** N/A — this is a build/runtime bug, not a scoring issue.
+- **Root cause:** `root.go.tmpl` line 100-104 iterates `.Resources` and emits AddCommand for each. Line 122-124 conditionally adds analytics from `.VisionSet.Analytics`. When an API has a `/analytics` endpoint AND a store-backed analytics command, both paths fire. The guard at line 101 checks `PromotedResourceNames` but not VisionSet names.
+- **Cross-API check:** Any API with an analytics endpoint (Stripe, Plausible, PostHog, Amplitude) would hit this. Any API where a resource name collides with a vision command name (analytics, export, import, search, sync, tail) would hit it.
+- **Frequency:** API subclass — APIs with endpoints named the same as vision commands. Common names: analytics, events, search.
+- **Fallback if not fixed:** Claude catches the panic on first build, removes the duplicate. But this is a 30-second delay on every affected API.
+- **Worth a Printing Press fix?** Yes — template fix is trivial.
+- **Inherent or fixable:** Fixable. Add VisionSet names to the Resources loop exclusion check.
+- **Durable fix:** In `root.go.tmpl`, expand the guard at line 101 to also skip resources whose name matches a VisionSet command: `{{- if and (not (index $.PromotedResourceNames $name)) (ne $name "auth") (not (index $.VisionSet $name))}}`. Or build a combined exclusion set in generator.go before passing to the template.
+- **Test:** Generate a CLI for an API with a `/analytics` endpoint. Verify root.go has exactly one `newAnalyticsCmd` registration.
+- **Evidence:** dub-pp-cli root.go had duplicate registration at lines 95 and 106.
+
+### F2. UpsertBatch doesn't populate resources_fts index (Bug)
+- **What happened:** `sync --full` wrote data to per-resource tables via `UpsertBatch`, but FTS search returned no results because `UpsertBatch` never populated `resources_fts`.
+- **Scorer correct?** Partially. Data Pipeline Integrity dropped from 10 to 7 after the manual fix because the search routing changed. The scorer is right that the data pipeline has an issue, but the original template is the root cause.
+- **Root cause:** `store.go.tmpl` line 334-362: `UpsertBatch` only does `INSERT OR REPLACE INTO resources` without the FTS `DELETE/INSERT` that `Upsert` (line 165-177) performs via `upsertGenericResourceTx`.
+- **Cross-API check:** Every CLI. UpsertBatch is the primary sync path.
+- **Frequency:** Every API.
+- **Fallback if not fixed:** Search silently returns empty results after sync. Claude may catch it during live testing, but only if the test includes a search step. High miss rate.
+- **Worth a Printing Press fix?** Yes — critical. Search is a headline feature.
+- **Inherent or fixable:** Fixable. Add FTS insert to UpsertBatch.
+- **Durable fix:** In `store.go.tmpl`, add FTS `DELETE` + `INSERT` into `resources_fts` inside the UpsertBatch transaction loop, same pattern as `upsertGenericResourceTx`.
+- **Test:** Generate any CLI, run `sync --full`, then `search <term>`. Must return results.
+- **Evidence:** dub-pp-cli search returned empty after sync until manually fixed.
+
+### F3. Search command routes to per-table FTS that's never populated (Bug)
+- **What happened:** The search command queried `links_fts`, `folders_fts`, `partners_fts` — but sync (via UpsertBatch) only populated `resources_fts`. Per-table FTS tables were always empty.
+- **Scorer correct?** The scorer wants domain-specific search methods. The per-table FTS tables exist and have correct schemas, but they're never written to. Scorer is partially right — the CLI should use per-table FTS, but the tables need to be populated.
+- **Root cause:** `search.go.tmpl` lines 171-176 wire search to per-table FTS methods. `sync.go.tmpl` line 237 uses `UpsertBatch`, which doesn't populate per-table FTS. The per-table `Upsert{{pascal .Name}}()` methods (store.go.tmpl lines 274-327) DO handle per-table FTS, but sync never calls them.
+- **Cross-API check:** Every CLI with FTS-enabled tables.
+- **Frequency:** Every API.
+- **Fallback if not fixed:** Same as F2 — search silently fails.
+- **Worth a Printing Press fix?** Yes. Two options: (a) sync uses per-table Upsert methods instead of generic UpsertBatch, or (b) UpsertBatch populates per-table FTS. Option (a) is more correct but requires sync to know about table names. Option (b) is simpler.
+- **Inherent or fixable:** Fixable. Best fix is making sync.go.tmpl call per-table upsert methods when the resource matches a known table, falling back to UpsertBatch for unknown resources.
+- **Durable fix:** In `sync.go.tmpl`, add a dispatch switch: for resources with per-table FTS, call `db.Upsert{{pascal resource}}()` per item. For all others, continue with `UpsertBatch`. This also populates per-table FTS correctly.
+- **Test:** Generate CLI, sync, search via per-table FTS. Must return results.
+- **Evidence:** dub-pp-cli search returned "no such column" errors from empty FTS tables.
+
+### F4. retryAfter has no maximum cap (Bug)
+- **What happened:** Dub's API returned a Retry-After header with a far-future timestamp, causing `retryAfter()` to return ~1.2 million hours. Sync hung indefinitely.
+- **Scorer correct?** N/A — not a scored dimension.
+- **Root cause:** `client.go.tmpl` lines 547-562: `retryAfter()` parses Retry-After as seconds or HTTP date but applies no maximum. Both `time.Duration(seconds) * time.Second` and `time.Until(t)` can return enormous values.
+- **Cross-API check:** Any API that sends Retry-After headers with large values or dates. Common with rate-limited APIs.
+- **Frequency:** Most APIs — any API with rate limiting.
+- **Fallback if not fixed:** CLI hangs during sync. User must Ctrl-C. Claude may not catch it because the hang happens during runtime, not build.
+- **Worth a Printing Press fix?** Yes — simple, high impact.
+- **Inherent or fixable:** Fixable. Cap at 60 seconds.
+- **Durable fix:** In `client.go.tmpl`, add `const maxRetryWait = 60 * time.Second` and cap both the integer and date parsing paths.
+- **Test:** Mock a 429 response with `Retry-After: 999999`. Verify wait is capped at 60s.
+- **Evidence:** dub-pp-cli sync hung for "1279358h2m3s" on events endpoint.
+
+### F5. Dogfood checks client.go for "Bearer " but auth is in config.go (Scorer bug)
+- **What happened:** Dogfood reported "auth protocol mismatch: uses unknown prefix" even though the CLI correctly implements Bearer auth. Had to add a dummy constant `authScheme = "Bearer "` to client.go.
+- **Scorer correct?** No. The CLI is correct. Dogfood checks the wrong file.
+- **Root cause:** `dogfood.go` lines 459-474 reads `client.go` and searches for `"Bearer "` or `"Bot "` string literals. But the generated client delegates auth to `config.AuthHeader()`, which is in `config.go`. The string `"Bearer "` only appears in config.go.tmpl (lines 115, 127).
+- **Cross-API check:** Every CLI using Bearer auth (most APIs).
+- **Frequency:** Every API with bearer_token auth.
+- **Fallback if not fixed:** Claude adds a dummy constant to client.go. Works but is a workaround for a broken check.
+- **Worth a Printing Press fix?** Yes — fix dogfood to also check config.go.
+- **Inherent or fixable:** Fixable. Expand the file search in dogfood.go to include config.go.
+- **Durable fix:** In `dogfood.go` line 459, read both `client.go` AND `config.go`. Check both sources for the auth scheme string.
+- **Test:** Generate a Bearer auth CLI. Run dogfood without the dummy constant. Must report MATCH.
+- **Evidence:** dub-pp-cli dogfood reported FAIL until authScheme constant was added to client.go.
+
+### F6. formatCompact emitted but never called (Template gap)
+- **What happened:** `helpers.go` contained dead function `formatCompact`. Dogfood caught it. Had to manually delete.
+- **Scorer correct?** Yes — Dead Code check is correct.
+- **Root cause:** `helpers.go.tmpl` lines 693-705 unconditionally emit `formatCompact`. Zero call sites exist in any template.
+- **Cross-API check:** Every CLI.
+- **Frequency:** Every API.
+- **Fallback if not fixed:** Claude deletes it during shipcheck. Reliable but wasteful — 5-10 seconds per generation.
+- **Worth a Printing Press fix?** Yes — remove from template or gate behind a conditional.
+- **Inherent or fixable:** Fixable. Remove the function from the template, or gate it behind a `{{if .UsesCompactNumbers}}` conditional.
+- **Durable fix:** Remove `formatCompact` from `helpers.go.tmpl`. If a future feature needs it, add it back with a gate.
+- **Test:** Generate any CLI. Verify helpers.go has no `formatCompact` function. Run `go vet` — no dead code.
+- **Evidence:** dub-pp-cli helpers.go had unused formatCompact, caught by dogfood.
+
+## Prioritized Improvements
+
+### P1 — High priority
+| Finding | Title | Component | Frequency | Fallback Reliability | Complexity |
+|---------|-------|-----------|-----------|---------------------|------------|
+| F2 | UpsertBatch missing FTS | Generator: store.go.tmpl | Every API | Low — search silently fails | Small |
+| F3 | Search routes to dead FTS | Generator: search.go.tmpl + sync.go.tmpl | Every API | Low — search silently fails | Medium |
+| F4 | retryAfter no cap | Generator: client.go.tmpl | Most APIs | Low — CLI hangs | Small |
+| F5 | Dogfood checks wrong file | Scorer: dogfood.go | Every bearer API | Medium — workaround exists | Small |
+
+### P2 — Medium priority
+| Finding | Title | Component | Frequency | Fallback Reliability | Complexity |
+|---------|-------|-----------|-----------|---------------------|------------|
+| F1 | Duplicate AddCommand | Generator: root.go.tmpl | API subclass (resource name = vision name) | High — caught on build | Small |
+| F6 | formatCompact dead code | Generator: helpers.go.tmpl | Every API | High — caught by dogfood | Small |
+
+### Skip
+*No findings skipped — all are systemic.*
+
+## Work Units
+
+### WU-1: Fix FTS indexing in UpsertBatch and sync routing (from F2, F3)
+- **Goal:** Search returns results after sync for every generated CLI
+- **Target:** Generator templates: `internal/generator/templates/store.go.tmpl` and `internal/generator/templates/sync.go.tmpl`
+- **Acceptance criteria:**
+ - positive: Generate CLI, sync, search — returns results from both generic and per-table FTS
+ - negative: Per-table FTS tables contain data after UpsertBatch
+- **Scope boundary:** Does not change the FTS schema or add new FTS tables
+- **Dependencies:** None
+- **Complexity:** Medium
+
+### WU-2: Cap retryAfter at 60 seconds (from F4)
+- **Goal:** CLI never waits more than 60 seconds on a rate limit response
+- **Target:** Generator template: `internal/generator/templates/client.go.tmpl` lines 547-562
+- **Acceptance criteria:**
+ - positive: 429 with Retry-After: 999999 waits 60s, not 999999s
+ - negative: 429 with Retry-After: 5 still waits 5s
+- **Scope boundary:** Does not change retry count or backoff strategy
+- **Dependencies:** None
+- **Complexity:** Small
+
+### WU-3: Fix dogfood Bearer detection to check config.go (from F5)
+- **Goal:** Dogfood correctly detects Bearer auth when constructed in config.go
+- **Target:** Scorer: `internal/pipeline/dogfood.go` lines 459-474
+- **Acceptance criteria:**
+ - positive: Bearer auth CLI passes dogfood without dummy constant in client.go
+ - negative: Non-bearer auth CLIs are not false-positively matched
+- **Scope boundary:** Does not change scorecard auth scoring
+- **Dependencies:** None
+- **Complexity:** Small
+
+### WU-4: Deduplicate VisionSet vs Resource AddCommand in root.go (from F1)
+- **Goal:** No duplicate AddCommand registrations when resource name matches a vision command
+- **Target:** Generator template: `internal/generator/templates/root.go.tmpl` lines 100-104
+- **Acceptance criteria:**
+ - positive: API with /analytics endpoint generates one newAnalyticsCmd registration
+ - negative: API without /analytics still gets VisionSet analytics
+- **Scope boundary:** Does not change promoted command logic
+- **Dependencies:** None
+- **Complexity:** Small
+
+### WU-5: Remove dead formatCompact from helpers template (from F6)
+- **Goal:** No dead functions emitted by default
+- **Target:** Generator template: `internal/generator/templates/helpers.go.tmpl` lines 693-705
+- **Acceptance criteria:**
+ - positive: Generated helpers.go has no formatCompact
+ - negative: go vet reports no unused functions in helpers.go
+- **Scope boundary:** If formatCompact is needed later, add it back with a conditional gate
+- **Dependencies:** None
+- **Complexity:** Small
+
+## Anti-patterns
+- **Workaround over root cause:** Adding `authScheme = "Bearer "` to client.go is a workaround for a dogfood bug. The fix should be in dogfood.go.
+- **Silent failure:** FTS search returning empty results without error is the worst kind of bug — it looks like "no data" rather than "broken index."
+
+## What the Printing Press Got Right
+- **OpenAPI parsing:** 47 operations across 14 resources parsed correctly on first try. Zero path validity failures.
+- **Auth detection:** Bearer token auth inferred correctly from spec security scheme. Doctor validates credentials.
+- **Quality gates:** All 7 gates passed on first pull (go mod tidy, go vet, go build, binary, --help, version, doctor).
+- **Agent-native output:** --json, --select, --csv, --compact, --dry-run, --agent all worked correctly across all commands.
+- **Scorecard 96/100 pre-polish:** Only 4 points lost, all in domain correctness tier.
+- **Store schema generation:** 15 tables with appropriate FTS indexes generated from spec analysis.
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 9c39c1e6..e0cca208 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -676,6 +676,7 @@ func (g *Generator) Generate() error {
rootData := struct {
*spec.APISpec
VisionSet VisionTemplateSet
+ VisionCmdNames map[string]bool
WorkflowConstructors []string
InsightConstructors []string
PromotedCommands []PromotedCommand
@@ -683,6 +684,7 @@ func (g *Generator) Generate() error {
}{
APISpec: g.Spec,
VisionSet: g.VisionSet,
+ VisionCmdNames: g.VisionSet.CmdNames(),
WorkflowConstructors: renderedWorkflowConstructors,
InsightConstructors: renderedInsightConstructors,
PromotedCommands: promotedCommands,
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index c755e18f..871b6bed 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -544,16 +544,25 @@ func (c *Client) refreshAccessToken() error {
return nil
}
+const maxRetryWait = 60 * time.Second
+
func retryAfter(resp *http.Response) time.Duration {
header := resp.Header.Get("Retry-After")
if header == "" {
return 5 * time.Second
}
if seconds, err := strconv.Atoi(header); err == nil {
- return time.Duration(seconds) * time.Second
+ d := time.Duration(seconds) * time.Second
+ if d > maxRetryWait {
+ return maxRetryWait
+ }
+ return d
}
if t, err := http.ParseTime(header); err == nil {
wait := time.Until(t)
+ if wait > maxRetryWait {
+ return maxRetryWait
+ }
if wait > 0 {
return wait
}
diff --git a/internal/generator/templates/helpers.go.tmpl b/internal/generator/templates/helpers.go.tmpl
index 878887dc..51c27f29 100644
--- a/internal/generator/templates/helpers.go.tmpl
+++ b/internal/generator/templates/helpers.go.tmpl
@@ -690,20 +690,6 @@ func wantsHumanTable(w io.Writer, flags *rootFlags) bool {
return isTerminal(w)
}
-// formatCompact formats large numbers compactly (e.g., 1.2M, 728K).
-func formatCompact(n int64) string {
- switch {
- case n >= 1_000_000:
- return fmt.Sprintf("%.1fM", float64(n)/1_000_000)
- case n >= 10_000:
- return fmt.Sprintf("%.0fK", float64(n)/1_000)
- case n >= 1_000:
- return fmt.Sprintf("%.1fK", float64(n)/1_000)
- default:
- return fmt.Sprintf("%d", n)
- }
-}
-
func printAutoTable(w io.Writer, items []map[string]any) error {
if len(items) == 0 {
return nil
diff --git a/internal/generator/templates/root.go.tmpl b/internal/generator/templates/root.go.tmpl
index 5cae5eda..61d4a198 100644
--- a/internal/generator/templates/root.go.tmpl
+++ b/internal/generator/templates/root.go.tmpl
@@ -98,7 +98,7 @@ func Execute() error {
}
{{- range $name, $resource := .Resources}}
-{{- if and (not (index $.PromotedResourceNames $name)) (ne $name "auth")}}
+{{- if and (not (index $.PromotedResourceNames $name)) (ne $name "auth") (not (index $.VisionCmdNames $name))}}
rootCmd.AddCommand(new{{camel $name}}Cmd(&flags))
{{- end}}
{{- end}}
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index 75a4a0b7..86179fa1 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -356,6 +356,17 @@ func (s *Store) UpsertBatch(resourceType string, items []json.RawMessage) error
if err != nil {
return fmt.Errorf("upserting %s/%s: %w", resourceType, id, err)
}
+
+ // Update FTS index (non-fatal — matches upsertGenericResourceTx pattern)
+ if _, ftsErr := tx.Exec(`DELETE FROM resources_fts WHERE id = ?`, id); 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 (id, resource_type, content) VALUES (?, ?, ?)`,
+ id, resourceType, string(item),
+ ); ftsErr != nil {
+ fmt.Fprintf(os.Stderr, "warning: FTS index update failed for %s/%s: %v\n", resourceType, id, ftsErr)
+ }
}
return tx.Commit()
diff --git a/internal/generator/vision_templates.go b/internal/generator/vision_templates.go
index 1c8e65cf..69c14e4a 100644
--- a/internal/generator/vision_templates.go
+++ b/internal/generator/vision_templates.go
@@ -18,6 +18,32 @@ type VisionTemplateSet struct {
Insights []string
}
+// CmdNames returns a set of command names that the VisionSet registers in
+// root.go. Used to exclude these from the Resources loop to prevent duplicates
+// when an API has an endpoint with the same name (e.g., /analytics).
+func (s VisionTemplateSet) CmdNames() map[string]bool {
+ names := map[string]bool{}
+ if s.Export {
+ names["export"] = true
+ }
+ if s.Import {
+ names["import"] = true
+ }
+ if s.Search {
+ names["search"] = true
+ }
+ if s.Sync {
+ names["sync"] = true
+ }
+ if s.Tail {
+ names["tail"] = true
+ }
+ if s.Analytics {
+ names["analytics"] = true
+ }
+ return names
+}
+
func (s VisionTemplateSet) IsZero() bool {
return !s.Export && !s.Import && !s.Store && !s.Search &&
!s.Sync && !s.Tail && !s.Analytics && !s.MCP &&
diff --git a/internal/pipeline/dogfood.go b/internal/pipeline/dogfood.go
index 71e5dde6..e957a897 100644
--- a/internal/pipeline/dogfood.go
+++ b/internal/pipeline/dogfood.go
@@ -463,11 +463,15 @@ func checkAuth(dir string, auth apispec.AuthConfig) AuthCheckResult {
return result
}
- clientSource := string(clientData)
+ // Also read config.go — Bearer/Bot prefix may be constructed there
+ // rather than in client.go (e.g., config.AuthHeader() returns "Bearer " + token).
+ configData, _ := os.ReadFile(filepath.Join(dir, "internal", "config", "config.go"))
+
+ combinedSource := string(clientData) + string(configData)
switch {
- case strings.Contains(clientSource, `"Bot "`):
+ case strings.Contains(combinedSource, `"Bot "`):
result.GeneratedFmt = "Bot "
- case strings.Contains(clientSource, `"Bearer "`):
+ case strings.Contains(combinedSource, `"Bearer "`):
result.GeneratedFmt = "Bearer "
default:
result.GeneratedFmt = "unknown"
← 0e30fc53 chore(main): release 1.1.0 (#134)
·
back to Cli Printing Press
·
feat(cli): MCP readiness layer — per-endpoint auth awareness 51afd778 →