Uh oh!
There was an error while loading. Please reload this page.
ci: file-budget line-count ratchet (scripts/file-budget.sh) wired into make ci + build.yml - #316
Merged
saadqbal merged 4 commits intoJul 14, 2026
Conversation
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 55 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
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 8fc62f8. Configure here.
saadqbal
approved these changes
Jul 14, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
👍 Same solid ratchet pattern as coverage-floor.sh — fail-loud on malformed/missing entries, clear 'split it or raise the ceiling' remediation, and CI green so every budgeted file is within budget. (home.go at 850 is a tight ceiling given it's ~841 now — deliberate nudge toward splitting, noted.) (Stacked on #315.)
8 tasks
saadqbal
commented
Jul 14, 2026
Collaborator
…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>
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>
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>
saadqbalforce-pushed
the
refactor/284-exitcodes-naming
branch
from
July 14, 2026 14:43
a6de340 to
8d93e43Comparesaadqbalforce-pushed
the
ci/285-file-budget-ratchet
branch
from
July 14, 2026 14:43
8fc62f8 to
7868fe4CompareThere was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7868fe4. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
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>
saadqbalforce-pushed
the
refactor/284-exitcodes-naming
branch
from
July 14, 2026 15:55
8d93e43 to
f12fa87CompareUh oh!
There was an error while loading. Please reload this page.
8 tasks
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>
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
scripts/file-budget.sh— a per-file line-count ratchet cloning thescripts/coverage-floor.shpattern (read first, same shape):path:max_linespairs,wc -lcheck, bash-3.2-portable, 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 only ratchet down. Raising one requires editing the checked-in script — a reviewed diff, never a side effect of a big PR. Seeds sit just above today's reality (verified on the WS-B branch):
internal/push/preflight.gointernal/cli/data.gointernal/cli/client.gointernal/cli/home.goWiring:
make ci— newfile-budgettarget in the chain (the Makefile's "make ci green ⇒ CI green" invariant requires both sides, hence also:)build.yml— one surgical step appended to the existinglintjob (my brief assigns this shared-file edit; per the standing convention new jobs would get a dedicated workflow file, but this is a single step in an existing job). Expected trivial rebase vs sibling PRs touching build.yml/Makefile — merge order flexible.Shell ratchet over a golangci
funlen-style rule per the #6 standalone-tools decision.Fixes#285
Part of tracebloc/backend#1106.
Type of change
Test plan
Ran locally:
make file-budget— all four seeds pass::error::annotationshellcheck --severity=warning scripts/file-budget.sh— cleango build ./...+ unit tests for touched-adjacent packages (internal/cli,internal/push) — greenbuild.ymlparses as valid YAMLChecklist
citarget + one build.yml lint step)Stacked PR 4/4 of the WS-B data.go chain — merge order: cli#303 (#282) → cli#309 (#283) → cli#315 (#284) → this. Branch is based on
refactor/284-exitcodes-naming; until the predecessors merge, this diff shows their commits too. Review only the top commit (8fc62f8). Note: thedata.go:500seed assumes the split lands first — merging this out of order would still pass (500 > today's 240 either way), but the budget's rationale reads post-split.🤖 Generated with Claude Code
Note
Low Risk
CI/Makefile-only change with no runtime behavior; risk is limited to false positives if line counts or paths drift without updating the budget list.
Overview
Adds
scripts/file-budget.sh, a shell gate (same shape ascoverage-floor.sh) that fails CI when tracked files exceed checked-inpath:max_linesceilings. Ceilings are meant to ratchet down only; raising a limit requires an explicit edit to the script.Wiring: new
file-budgetMakefile target and inclusion inmake ci, plus a File budget step inbuild.yml’s lint job so localmake ciand CI stay aligned.Initial budgets:
internal/push/preflight.go,internal/cli/data.go,internal/cli/client.go, andinternal/cli/home.go. The script also errors on malformed budget entries or missing budgeted paths so renames don’t silently drop coverage.Reviewed by Cursor Bugbot for commit def16ab. Bugbot is set up for automated code reviews on this repo. Configure here.