Uh oh!
There was an error while loading. Please reload this page.
test(cli): pin prompt-validators + review renderers - #264
Merged
Conversation
…rvivors) Third follow-up from the mutation pass, on the cli side. These guard the last confirm gate before a destructive ingest/create, so a silently-dropped (or wrongly-shown) review field is a real regression. Add pure / buffer-only tests that each KILL a surviving mutant: - validatePositiveInt: "0" must be rejected (kills n <= 0 -> n < 0) - boundedInt: exactly lo and exactly hi are accepted (kills n < lo -> <=, n > hi -> >=; inclusive range) - renderReview: every optional field renders iff set, and the resolution/schema switches fall to the category-driven default when the flag is empty (kills the `!= ""` / `> 0` negation + IsImage/IsTabular switch mutants) - renderClientReview: location + cluster render only when non-empty 4 representative mutants mutation-proven (flip the operator -> the matching test fails). Skipped as equivalent/unkillable: autoClientName's DNS-cap truncation (no-op at the boundary for a slugified base) and validateDesired's runtime.GOOS platform guard. Pure additive test file; no production change. make ci green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9b8a1f7. Configure here.
LukasWodkaforce-pushed
the
test/cli-pure-validators
branch
from
July 14, 2026 08:03
afecfed to
9b8a1f7CompareLukasWodka
commented
Jul 14, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9b8a1f7. Configure here.
saadqbal
approved these changes
Jul 14, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
LGTM 👍 Good confirm-gate coverage — verified validatePositiveInt rejects "0" (the <=0 boundary), boundedInt is inclusive at both ends, and renderReview's field gating + the resolution/schema switch defaults (auto-detect / infer-from-CSV) match. The review panel is exactly the surface worth pinning before a destructive ingest.
Uh oh!
There was an error while loading. Please reload this page.
saadqbal pushed a commit
that referenced
this pull request
Jul 14, 2026
…unDataIngest runDataIngest was 586 lines; every pre-cluster step and the cluster pre-flight now live in two named stanza functions, moved verbatim: - resolveLocalInput (data_ingest_local.go) — steps 0–4 + the P3 content preflight: flag guard, banner, guided prompts, ~-expansion + existence-first check, table-name/category/misapplied-flag validation, layout walk, per-category spec resolution, spec synthesis + schema validation, local summary. Mutates a in place (via *runDataIngestArgs) so the --output-json defer and the cluster steps see the resolved spec exactly as before. - connectIngestTarget (data_ingest_cluster.go) — steps 5–8a: kubeconfig resolve, release + PVC discovery, verbose cluster summary, and the destination-table guard (incl. the folded interactive replace decision, which still flips a.Overwrite). The jsonEmitted/named-return defer dance stays INSIDE runDataIngest, next to the err it reads — both extracted functions feed that named return, so the error-JSON contract is unchanged. The dry-run stop (which sets jsonEmitted) also stays inside. Only in-stanza edits are what the new signatures force: multi-value returns on error paths, := to = where a named result already exists, and &a -> a where a is already a pointer. Step order, output order, and exit codes are untouched. data.go: 693 -> 240 lines; runDataIngest: 586 -> 138. Safety net: the #187 outcome-matrix table test + #264 mutation pins pass untouched; full suite green under -race; coverage floors hold (82.7%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
saadqbal added a commit
that referenced
this pull request
Jul 14, 2026
…unDataIngest (#321) runDataIngest was 586 lines; every pre-cluster step and the cluster pre-flight now live in two named stanza functions, moved verbatim: - resolveLocalInput (data_ingest_local.go) — steps 0–4 + the P3 content preflight: flag guard, banner, guided prompts, ~-expansion + existence-first check, table-name/category/misapplied-flag validation, layout walk, per-category spec resolution, spec synthesis + schema validation, local summary. Mutates a in place (via *runDataIngestArgs) so the --output-json defer and the cluster steps see the resolved spec exactly as before. - connectIngestTarget (data_ingest_cluster.go) — steps 5–8a: kubeconfig resolve, release + PVC discovery, verbose cluster summary, and the destination-table guard (incl. the folded interactive replace decision, which still flips a.Overwrite). The jsonEmitted/named-return defer dance stays INSIDE runDataIngest, next to the err it reads — both extracted functions feed that named return, so the error-JSON contract is unchanged. The dry-run stop (which sets jsonEmitted) also stays inside. Only in-stanza edits are what the new signatures force: multi-value returns on error paths, := to = where a named result already exists, and &a -> a where a is already a pointer. Step order, output order, and exit codes are untouched. data.go: 693 -> 240 lines; runDataIngest: 586 -> 138. Safety net: the #187 outcome-matrix table test + #264 mutation pins pass untouched; full suite green under -race; coverage floors hold (82.7%). Co-authored-by: Lukas Wuttke <lukas@tracebloc.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
saadqbal pushed a commit
that referenced
this pull request
Jul 14, 2026
…o make ci + build.yml (#316) * refactor(cli): extract resolveLocalInput + connectIngestTarget from runDataIngest runDataIngest was 586 lines; every pre-cluster step and the cluster pre-flight now live in two named stanza functions, moved verbatim: - resolveLocalInput (data_ingest_local.go) — steps 0–4 + the P3 content preflight: flag guard, banner, guided prompts, ~-expansion + existence-first check, table-name/category/misapplied-flag validation, layout walk, per-category spec resolution, spec synthesis + schema validation, local summary. Mutates a in place (via *runDataIngestArgs) so the --output-json defer and the cluster steps see the resolved spec exactly as before. - connectIngestTarget (data_ingest_cluster.go) — steps 5–8a: kubeconfig resolve, release + PVC discovery, verbose cluster summary, and the destination-table guard (incl. the folded interactive replace decision, which still flips a.Overwrite). The jsonEmitted/named-return defer dance stays INSIDE runDataIngest, next to the err it reads — both extracted functions feed that named return, so the error-JSON contract is unchanged. The dry-run stop (which sets jsonEmitted) also stays inside. Only in-stanza edits are what the new signatures force: multi-value returns on error paths, := to = where a named result already exists, and &a -> a where a is already a pointer. Step order, output order, and exit codes are untouched. data.go: 693 -> 240 lines; runDataIngest: 586 -> 138. Safety net: the #187 outcome-matrix table test + #264 mutation pins pass untouched; full suite green under -race; coverage floors hold (82.7%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(cli): name every exit-code site + cross-command docs table internal/cli/exitcodes.go names every exit code the CLI produces; every non-test &exitError construction site now uses a named constant instead of a bare number. exit.go keeps owning the extraction — this is a naming sweep, not a redesign; all numeric values are unchanged (and documented as frozen: they're the scripting contract customers branch on). Site census (the ticket's count of 138 grep hits includes one doc comment in exit.go; 137 are real construction sites): 61x1, 25x2, 23x3, 7x4, 4x5, 1x6, 5x7, 2x8, 4x9, 2x130, 2x kubeconfigExitCode(...) (the helper now returns named constants), 1x variable (runLocalPreflight's BadFlag fold, now built from constants). Zero numeric literals remain at non-test sites. Codes that grew more than one per-command meaning get one constant per MEANING sharing the value (2: exitBadInput / doctor's exitChecksFailed; 5: exitAuth / data delete's exitNoSuchDataset; 7: exitStagingFailed / exitTeardownFailed / exitQueryFailed), so each site reads honestly. docs/troubleshooting.md gains the cross-command exit-code table (code, meaning, producing commands, constant name), sourced from the per-command long-help blocks — data ingest's (data_ingest_cmd.go, pre-split data.go:236-252) is the fullest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: file-budget line-count ratchet (scripts/file-budget.sh) Clones the coverage-floor.sh pattern for file size: path:max_lines pairs, checked by wc -l, portable to bash 3.2, with the same malformed-entry guard — plus a missing-file guard so a moved/renamed budgeted file makes the stale list loud instead of silently un-budgeting it. Ceilings are a RATCHET: only ever lowered as files shrink; raising one is a deliberate, reviewed edit to the checked-in script. Seeds sit just above today's reality (develop + the WS-B split): internal/push/preflight.go:1650 (now 1635) internal/cli/data.go:500 (now 240 post cli#282/#283) internal/cli/client.go:1050 (now 1027) internal/cli/home.go:850 (now 841) Wired into `make ci` (new file-budget target, keeping the Makefile's "make ci green => CI green" invariant) and into build.yml's lint job as one surgical step. Exercised all three failure paths locally (over budget, missing file, malformed entry) — each exits 1 with a ::error:: annotation; shellcheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(file-budget): seed preflight.go at true size (Bugbot #316 HIGH) The ratchet seeded internal/push/preflight.go at 1650, but the file is 1655 lines (wc -l, same count the gate uses) and the check is strict- greater — so `make file-budget` would fail the moment this landed on develop. Raise the ceiling to 1700, the next round-50 ratchet value just above actual, matching how the other seeds sit above their files (client.go 1050>1027, home.go 850>849, data.go 500>240). Also correct the header comment to reality (preflight ~1655, home ~849); it wrongly read ~1635 / ~841. Proof: `bash scripts/file-budget.sh` now passes (preflight 1655 <= 1700). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
saadqbal pushed a commit
that referenced
this pull request
Jul 14, 2026
…mand docs table (#315) * refactor(cli): name every exit-code site + cross-command docs table internal/cli/exitcodes.go names every exit code the CLI produces; every non-test &exitError construction site now uses a named constant instead of a bare number. exit.go keeps owning the extraction — this is a naming sweep, not a redesign; all numeric values are unchanged (and documented as frozen: they're the scripting contract customers branch on). Site census (the ticket's count of 138 grep hits includes one doc comment in exit.go; 137 are real construction sites): 61x1, 25x2, 23x3, 7x4, 4x5, 1x6, 5x7, 2x8, 4x9, 2x130, 2x kubeconfigExitCode(...) (the helper now returns named constants), 1x variable (runLocalPreflight's BadFlag fold, now built from constants). Zero numeric literals remain at non-test sites. Codes that grew more than one per-command meaning get one constant per MEANING sharing the value (2: exitBadInput / doctor's exitChecksFailed; 5: exitAuth / data delete's exitNoSuchDataset; 7: exitStagingFailed / exitTeardownFailed / exitQueryFailed), so each site reads honestly. docs/troubleshooting.md gains the cross-command exit-code table (code, meaning, producing commands, constant name), sourced from the per-command long-help blocks — data ingest's (data_ingest_cmd.go, pre-split data.go:236-252) is the fullest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: file-budget line-count ratchet (scripts/file-budget.sh) wired into make ci + build.yml (#316) * refactor(cli): extract resolveLocalInput + connectIngestTarget from runDataIngest runDataIngest was 586 lines; every pre-cluster step and the cluster pre-flight now live in two named stanza functions, moved verbatim: - resolveLocalInput (data_ingest_local.go) — steps 0–4 + the P3 content preflight: flag guard, banner, guided prompts, ~-expansion + existence-first check, table-name/category/misapplied-flag validation, layout walk, per-category spec resolution, spec synthesis + schema validation, local summary. Mutates a in place (via *runDataIngestArgs) so the --output-json defer and the cluster steps see the resolved spec exactly as before. - connectIngestTarget (data_ingest_cluster.go) — steps 5–8a: kubeconfig resolve, release + PVC discovery, verbose cluster summary, and the destination-table guard (incl. the folded interactive replace decision, which still flips a.Overwrite). The jsonEmitted/named-return defer dance stays INSIDE runDataIngest, next to the err it reads — both extracted functions feed that named return, so the error-JSON contract is unchanged. The dry-run stop (which sets jsonEmitted) also stays inside. Only in-stanza edits are what the new signatures force: multi-value returns on error paths, := to = where a named result already exists, and &a -> a where a is already a pointer. Step order, output order, and exit codes are untouched. data.go: 693 -> 240 lines; runDataIngest: 586 -> 138. Safety net: the #187 outcome-matrix table test + #264 mutation pins pass untouched; full suite green under -race; coverage floors hold (82.7%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(cli): name every exit-code site + cross-command docs table internal/cli/exitcodes.go names every exit code the CLI produces; every non-test &exitError construction site now uses a named constant instead of a bare number. exit.go keeps owning the extraction — this is a naming sweep, not a redesign; all numeric values are unchanged (and documented as frozen: they're the scripting contract customers branch on). Site census (the ticket's count of 138 grep hits includes one doc comment in exit.go; 137 are real construction sites): 61x1, 25x2, 23x3, 7x4, 4x5, 1x6, 5x7, 2x8, 4x9, 2x130, 2x kubeconfigExitCode(...) (the helper now returns named constants), 1x variable (runLocalPreflight's BadFlag fold, now built from constants). Zero numeric literals remain at non-test sites. Codes that grew more than one per-command meaning get one constant per MEANING sharing the value (2: exitBadInput / doctor's exitChecksFailed; 5: exitAuth / data delete's exitNoSuchDataset; 7: exitStagingFailed / exitTeardownFailed / exitQueryFailed), so each site reads honestly. docs/troubleshooting.md gains the cross-command exit-code table (code, meaning, producing commands, constant name), sourced from the per-command long-help blocks — data ingest's (data_ingest_cmd.go, pre-split data.go:236-252) is the fullest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: file-budget line-count ratchet (scripts/file-budget.sh) Clones the coverage-floor.sh pattern for file size: path:max_lines pairs, checked by wc -l, portable to bash 3.2, with the same malformed-entry guard — plus a missing-file guard so a moved/renamed budgeted file makes the stale list loud instead of silently un-budgeting it. Ceilings are a RATCHET: only ever lowered as files shrink; raising one is a deliberate, reviewed edit to the checked-in script. Seeds sit just above today's reality (develop + the WS-B split): internal/push/preflight.go:1650 (now 1635) internal/cli/data.go:500 (now 240 post cli#282/#283) internal/cli/client.go:1050 (now 1027) internal/cli/home.go:850 (now 841) Wired into `make ci` (new file-budget target, keeping the Makefile's "make ci green => CI green" invariant) and into build.yml's lint job as one surgical step. Exercised all three failure paths locally (over budget, missing file, malformed entry) — each exits 1 with a ::error:: annotation; shellcheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(file-budget): seed preflight.go at true size (Bugbot #316 HIGH) The ratchet seeded internal/push/preflight.go at 1650, but the file is 1655 lines (wc -l, same count the gate uses) and the check is strict- greater — so `make file-budget` would fail the moment this landed on develop. Raise the ceiling to 1700, the next round-50 ratchet value just above actual, matching how the other seeds sit above their files (client.go 1050>1027, home.go 850>849, data.go 500>240). Also correct the header comment to reality (preflight ~1655, home ~849); it wrongly read ~1635 / ~841. Proof: `bash scripts/file-budget.sh` now passes (preflight 1655 <= 1700). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
saadqbal pushed a commit
that referenced
this pull request
Jul 14, 2026
* test: SwapSeam helper + advisory gremlins mutation workflow internal/testutil.SwapSeam(t, ptr, stub) replaces the hand-rolled save/stub/restore blocks around package-level seam variables: swap, then LIFO-restore via t.Cleanup. Generic, so non-function seams (timeouts) work too. Converted the three blocks in internal/submit (watch_test.go x2, run_watch_test.go x1); internal/push has none. The ~26 blocks in internal/cli are deliberately left for the decomposition work to pick up; converting them here would collide. mutation.yml formalizes the gremlins ritual that produced #262-#264: workflow_dispatch only, one package per run, advisory (lived mutants never fail the job — no thresholds). Prints a survivors-to-triage summary, uploads the JSON report, and CONTRIBUTING.md documents how survivors get triaged into pinning-test issues. Fixes#295 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: harden gremlins mutation workflow (Bugbot #306) - Reject a zero timeout-coefficient. The digit check accepted `0`/`00`, which collapses gremlins' per-mutant timeout so survivors report TIMED OUT instead of KILLED/LIVED — the exact failure mode this input guards against. Now require a positive integer (> 0), matching the error text. - Add `set -o pipefail` to the run step. The implicit default shell is `bash -e {0}` (no pipefail), so `gremlins | tee` masked a non-zero gremlins exit behind tee's 0, letting a broken suite pass the job — contradicting the workflow's "tool/test health fails the run" contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third follow-up from the mutation-testing pass, now on the
cliside. These functions guard the last confirm gate before a destructivedata ingest/client create, so a review field that silently stops rendering (or renders when it shouldn't) is a real regression. Pure / buffer-only tests — no cobra harness — each killing a surviving mutant:validatePositiveIntn <= 0→n < 0"0"is rejected (a positive int is> 0)boundedIntn < lo→<=,n > hi→>=loand exactlyhiare accepted (inclusive range)renderReview!= ""/> 0negations +IsImage/IsTabularswitchrenderClientReviewlocation != ""/clusterID != ""Mutation-proven: 4 representative mutants flipped in-source, each confirmed to fail the matching test.
Deliberately not chased (equivalent / unkillable):
autoClientName's DNS-cap truncation (a no-op at the exact boundary for a slugified base, which never ends in-) andvalidateDesired'sruntime.GOOS == "darwin"platform guard (not portably testable without an OS seam).Test plan
make cigreen (build, vet, gofmt -s,go test -race, errcheck/ineffassign/misspell, schema-check). Pure additive test file; no production change.🤖 Generated with Claude Code
Note
Low Risk
Additive tests only; no runtime or security behavior changes.
Overview
Adds
internal/cli/render_and_validators_test.gowith no production changes—pure test coverage aimed at gremlins survivors on the last confirm gate before destructivedata ingest/client create.validatePositiveIntandboundedInttests pin inclusive/exclusive boundaries (e.g."0"rejected for positive ints; endpointslo/hiaccepted for bounded ranges).renderReviewandrenderClientReviewtests assert optional fields render only when set, and that image/tabular categories show the right default copy when resolution/schema flags are empty.Reviewed by Cursor Bugbot for commit 9b8a1f7. Bugbot is set up for automated code reviews on this repo. Configure here.