← back to Cli Printing Press
fix(cli): add client-call reimplementation directive (#562)
6242e1011630ba45f94d316bb03ab739c284001e · 2026-05-03 21:27:29 -0700 · Trevin Chow
Files touched
M AGENTS.mdM internal/pipeline/reimplementation_check.goM internal/pipeline/reimplementation_check_test.goM skills/printing-press/references/absorb-scoring.md
Diff
commit 6242e1011630ba45f94d316bb03ab739c284001e
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun May 3 21:27:29 2026 -0700
fix(cli): add client-call reimplementation directive (#562)
---
AGENTS.md | 3 +-
internal/pipeline/reimplementation_check.go | 38 ++++++++---
internal/pipeline/reimplementation_check_test.go | 73 ++++++++++++++++++++++
skills/printing-press/references/absorb-scoring.md | 2 +-
4 files changed, 106 insertions(+), 10 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 96185ed7..f1f20638 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -33,8 +33,9 @@ Carve-outs:
- Commands that bypass the store package and operate on the local SQLite file directly through `database/sql` (import + `sql.Open`/`sql.OpenDB`) — same data, thinner surface
- Commands that cache an API response in the store after calling it — both a client call and a store call is fine
- Commands whose data is the curated content itself (substitution tables, holiday lists, currency metadata) — opt in via `// pp:novel-static-reference` directive in the command's source file
+- Commands that call the real API through a wrapper the dogfood heuristic cannot see — opt in via `// pp:client-call` directive in the command's source file, and use it only when the hidden helper performs a real external API call. Do not use it for hardcoded payloads, local-only transforms, or fake endpoint stubs.
-Enforced by the absorb manifest's Kill Check (`skills/printing-press/references/absorb-scoring.md`) and dogfood's `reimplementation_check`, which flags handler files showing neither a client call nor a store access (without the static-reference opt-out).
+Enforced by the absorb manifest's Kill Check (`skills/printing-press/references/absorb-scoring.md`) and dogfood's `reimplementation_check`, which flags handler files showing neither a client call nor a store access (without the static-reference or client-call opt-out).
## Agent-Native Surface
diff --git a/internal/pipeline/reimplementation_check.go b/internal/pipeline/reimplementation_check.go
index 413022e4..2dde2d4b 100644
--- a/internal/pipeline/reimplementation_check.go
+++ b/internal/pipeline/reimplementation_check.go
@@ -45,6 +45,13 @@ type ReimplementationCheckResult struct {
// dogfood-results.json can distinguish the two carve-out classes
// even though they share the same ship/no-ship decision.
ExemptedViaAnnotation int `json:"exempted_via_annotation,omitempty"`
+ // ExemptedViaClientDirective is the number of commands that passed
+ // via the // pp:client-call marker. This marker is a positive assertion
+ // that the command reaches a real API through a wrapper the heuristic
+ // cannot see. Tracked separately from static-reference annotations so
+ // dogfood analytics can distinguish "real API through abstraction" from
+ // "curated static data."
+ ExemptedViaClientDirective int `json:"exempted_via_client_directive,omitempty"`
// Suspicious is the list of commands whose files show no client
// call and no store access - the candidate hand-rolled responses.
Suspicious []ReimplementationFinding `json:"suspicious,omitempty"`
@@ -139,10 +146,8 @@ var (
)
// checkReimplementation scans the files that implement built novel
-// features and classifies each. A command whose file calls the store
-// package is exempt. A command whose file calls the client is fine. A
-// command whose file does neither - or whose handler is a trivial stub
-// - is flagged for review.
+// features and classifies each via classifyReimplementation's ordered
+// signal rules.
//
// When researchDir is empty or research.json has no novel features the
// check returns Skipped. This mirrors the behavior of checkNovelFeatures:
@@ -222,6 +227,9 @@ func checkReimplementation(cliDir, researchDir string) ReimplementationCheckResu
case exemptAnnotation:
result.ExemptedViaAnnotation++
continue
+ case exemptClientDirective:
+ result.ExemptedViaClientDirective++
+ continue
}
if !ok {
finding.Command = nf.Command
@@ -252,6 +260,13 @@ func checkReimplementation(cliDir, researchDir string) ReimplementationCheckResu
// Added for retro #301 finding F3.
var novelStaticReferenceRe = regexp.MustCompile(`(?m)^\s*//\s*pp:novel-static-reference\b`)
+// clientCallDirectiveRe matches the positive assertion marker for command
+// files that reach a real API through a wrapper the string heuristics cannot
+// see. Unlike pp:novel-static-reference, this is not a carve-out for static
+// data; it says "the API call exists, but not in a shape this checker can
+// verify mechanically."
+var clientCallDirectiveRe = regexp.MustCompile(`(?m)^\s*//\s*pp:client-call\b`)
+
// exemptionKind labels which carve-out vindicated a command, so the
// caller can route the bump to the right counter on
// ReimplementationCheckResult. exemptNone covers both "passes via
@@ -264,6 +279,7 @@ const (
exemptNone exemptionKind = iota
exemptStore
exemptAnnotation
+ exemptClientDirective
)
// classifyReimplementation returns the best classification across the
@@ -272,14 +288,17 @@ const (
// 1. If any file carries the `// pp:novel-static-reference` marker,
// the command is exempted as an intentional static-data feature.
// Return (_, exemptAnnotation, true).
-// 2. If any file shows a store signal, the command is exempted as a
+// 2. If any file carries the `// pp:client-call` marker, the command
+// is exempted as a real API call hidden behind an abstraction.
+// Return (_, exemptClientDirective, true).
+// 3. If any file shows a store signal, the command is exempted as a
// local-SQLite feature. Return (_, exemptStore, true).
-// 3. If any file shows a client signal, the command is fine. Return
+// 4. If any file shows a client signal, the command is fine. Return
// (_, exemptNone, true).
-// 4. Otherwise the command is suspicious. Return a ReimplementationFinding
+// 5. Otherwise the command is suspicious. Return a ReimplementationFinding
// naming the primary file and a reason. Return (finding, exemptNone, false).
//
-// The trivial-body regex is consulted only when rule 4 fires, to pick
+// The trivial-body regex is consulted only when rule 5 fires, to pick
// between "empty stub" and "hand-rolled response" as the reason.
func classifyReimplementation(files []string, fileContent map[string]string, storeHelpers map[string]bool) (ReimplementationFinding, exemptionKind, bool) {
hasClient := false
@@ -293,6 +312,9 @@ func classifyReimplementation(files []string, fileContent map[string]string, sto
if novelStaticReferenceRe.MatchString(content) {
return ReimplementationFinding{File: f}, exemptAnnotation, true
}
+ if clientCallDirectiveRe.MatchString(content) {
+ return ReimplementationFinding{File: f}, exemptClientDirective, true
+ }
if hasStoreSignal(content) {
return ReimplementationFinding{File: f}, exemptStore, true
}
diff --git a/internal/pipeline/reimplementation_check_test.go b/internal/pipeline/reimplementation_check_test.go
index a5975ec1..5ec301d1 100644
--- a/internal/pipeline/reimplementation_check_test.go
+++ b/internal/pipeline/reimplementation_check_test.go
@@ -583,6 +583,9 @@ func newSubCmd(flags *rootFlags) *cobra.Command {
if got.ExemptedViaAnnotation != 1 {
t.Fatalf("ExemptedViaAnnotation: want 1 (marker should exempt), got %d", got.ExemptedViaAnnotation)
}
+ if got.ExemptedViaClientDirective != 0 {
+ t.Fatalf("ExemptedViaClientDirective: want 0 (static marker is distinct), got %d", got.ExemptedViaClientDirective)
+ }
if got.ExemptedViaStore != 0 {
t.Errorf("ExemptedViaStore: want 0 (annotation is its own carve-out, not store), got %d", got.ExemptedViaStore)
}
@@ -591,6 +594,76 @@ func newSubCmd(flags *rootFlags) *cobra.Command {
}
}
+func TestCheckReimplementation_ClientCallMarker(t *testing.T) {
+ tests := []struct {
+ name string
+ marker string
+ wantDirective int
+ wantSuspicious int
+ }{
+ {
+ name: "marker exempts hidden client wrapper",
+ marker: `// pp:client-call
+//
+// fetchFlights wraps the real API client through a helper shape the
+// reimplementation regex cannot see.
+
+`,
+ wantDirective: 1,
+ },
+ {
+ name: "missing marker still flags hidden wrapper",
+ wantSuspicious: 1,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ files := map[string]string{
+ "flights.go": `package cli
+
+` + tt.marker + `import "github.com/spf13/cobra"
+
+func newFlightsCmd(flags *rootFlags) *cobra.Command {
+ return &cobra.Command{
+ Use: "flights",
+ RunE: func(cmd *cobra.Command, args []string) error {
+ flights, err := fetchFlights(args[0])
+ if err != nil { return err }
+ _ = flights
+ return nil
+ },
+ }
+}
+`,
+ }
+ cliDir, pipelineDir := seedReimplementationFixture(t, files, []NovelFeature{
+ {Name: "Flights", Command: "flights"},
+ })
+
+ got := checkReimplementation(cliDir, pipelineDir)
+ if got.Checked != 1 {
+ t.Fatalf("Checked: want 1, got %d", got.Checked)
+ }
+ if got.ExemptedViaClientDirective != tt.wantDirective {
+ t.Fatalf("ExemptedViaClientDirective: want %d, got %d", tt.wantDirective, got.ExemptedViaClientDirective)
+ }
+ if got.ExemptedViaAnnotation != 0 {
+ t.Fatalf("ExemptedViaAnnotation: want 0 (client-call marker is distinct), got %d", got.ExemptedViaAnnotation)
+ }
+ if got.ExemptedViaStore != 0 {
+ t.Fatalf("ExemptedViaStore: want 0 (client-call marker is not a store signal), got %d", got.ExemptedViaStore)
+ }
+ if len(got.Suspicious) != tt.wantSuspicious {
+ t.Fatalf("Suspicious: want %d, got %d (%v)", tt.wantSuspicious, len(got.Suspicious), got.Suspicious)
+ }
+ if tt.wantSuspicious > 0 && !strings.Contains(got.Suspicious[0].Reason, "no API client call") {
+ t.Errorf("expected hand-rolled-response reason, got %q", got.Suspicious[0].Reason)
+ }
+ })
+ }
+}
+
// TestCheckReimplementation_WithoutMarker_StillFlagged confirms the
// F3 fix doesn't silently exempt commands that lack the explicit
// `// pp:novel-static-reference` marker. Same shape as the test above
diff --git a/skills/printing-press/references/absorb-scoring.md b/skills/printing-press/references/absorb-scoring.md
index 8da6a1cd..5d168a93 100644
--- a/skills/printing-press/references/absorb-scoring.md
+++ b/skills/printing-press/references/absorb-scoring.md
@@ -22,7 +22,7 @@ be features that can actually ship.
| **Auth the user doesn't have** | Feature requires write access, OAuth scopes, or paid tiers the user hasn't confirmed | **Gate** behind an auth check, or **cut** if the feature is useless without it. Read-only features using the same auth as other commands are fine. |
| **Scope creep** | Feature is really an application, not a command. Would take >200 lines to implement, needs a TUI, or requires persistent background processes. | **Descope** to the one-command version. "Dashboard" → "summary stats." "Monitor" → "poll once with --watch." If the one-command version isn't useful, **cut**. |
| **Verifiability** | Feature can't be tested in dogfood. No way to verify the output is correct without manual inspection or domain expertise. | **Flag** as low-confidence. Keep only if the value is high enough to justify manual QA. |
-| **Reimplementation** | Feature synthesizes API responses locally instead of calling the API. Hand-rolled response builders, hardcoded JSON returned as an "API result," endpoint stubs that return constants, or aggregations computed in-process when the API has an aggregation endpoint. | **Cut or rewrite.** A printed CLI that pretends to call the API is strictly worse than the API call it replaces. The one exception is features that read from the local SQLite store (`stale`, `bottleneck`, `health`, `reconcile`); those are local-data commands, not fake API calls. Dogfood's `reimplementation_check` enforces this at generation time. |
+| **Reimplementation** | Feature synthesizes API responses locally instead of calling the API. Hand-rolled response builders, hardcoded JSON returned as an "API result," endpoint stubs that return constants, or aggregations computed in-process when the API has an aggregation endpoint. | **Cut or rewrite.** A printed CLI that pretends to call the API is strictly worse than the API call it replaces. Local SQLite commands (`stale`, `bottleneck`, `health`, `reconcile`) are local-data commands, not fake API calls. Curated static-reference commands must use `// pp:novel-static-reference`. Commands that call a real API through a wrapper dogfood cannot see must use `// pp:client-call`; never use that directive for hardcoded payloads or fake stubs. Dogfood's `reimplementation_check` enforces this at generation time. |
**Buildability proof.** For each surviving feature, write one sentence:
← 35fa7afe fix(cli): support OpenAPI auth metadata overrides (#561)
·
back to Cli Printing Press
·
fix(cli): exempt generated-client source helpers (#563) c0639cf0 →