Uh oh!
There was an error while loading. Please reload this page.
refactor(cli): extract resolveLocalInput + connectIngestTarget from runDataIngest - #309
Conversation
…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>
LukasWodka
commented
Jul 14, 2026
👋 Heads-up — Code review queue is at 48 / 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
@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 a2464db. Configure here.
LukasWodka
commented
Jul 14, 2026
@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 a2464db. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
👍 Careful extraction — passing *runDataIngestArgs correctly threads the in-place a.Spec/a.Overwrite mutations, the explicit (cancelled, err) returns preserve the interactive-cancel + error paths, and err feeding the named return keeps the reclaim defer firing. Test + all builds green = behavior preserved. (Stacked on #303.)
saadqbal
commented
Jul 14, 2026
Heads-up: this got auto-closed when I squash-merged its base #303 with branch deletion — GitHub closes a PR when its base branch is deleted, and it can't be reopened (base ref gone). Your work is safe on |
Summary
The risky one of the WS-B chain:
runDataIngestwas 586 lines; its two big stanzas are now named functions, moved verbatim:resolveLocalInput(→data_ingest_local.go) — steps 0–4 + the P3 content preflight: the--overwrite/--idempotency-keyguard, intro banner, guided prompts,~-expansion + the existence-first check, table-name/category/misapplied-flag validation, the layout walk, per-category spec resolution, spec synthesis + schema validation, and the local summary. Takes*runDataIngestArgsand mutates it in place, so the--output-jsonerror defer and the cluster steps see the resolved spec exactly as the inline code did.connectIngestTarget(→data_ingest_cluster.go) — steps 5–8a: kubeconfig resolve, release + shared-PVC discovery, the verbose cluster summary, and the destination-table guard including the folded interactive replace decision (still flipsa.Overwrite).The
jsonEmitted/named-return defer dance stays insiderunDataIngest, next to theerrit reads — both extracted functions feed that named return, and the dry-run stop (which setsjsonEmitted) also stays inside. The error-JSON contract is unchanged.The only in-stanza edits are what the new signatures force: multi-value returns on error paths,
:=→=where a named result already exists,&a→awhereais already a pointer. Step order, output order, and exit codes untouched.Targets hit:
data.go693 → 240 lines (<250 ✓);runDataIngest586 → 138 lines (<150 ✓).Fixes#283
Part of tracebloc/backend#1106.
Type of change
Test plan
Ran locally:
go test -race ./...— full suite green (14 packages, zero FAIL)TestRunIngestionRun_Matrix) + the test(cli): pin prompt-validators + review renderers #264 mutation pins (render_and_validators_test.go) — all pass; no test file modified in this PRscripts/coverage-floor.sh— internal/cli 82.7% ≥ 68, internal/submit 80.4% ≥ 72go build ./...,go vet,gofmt -s -l— cleanChecklist
Stacked PR 2/4 of the WS-B data.go chain — merge order: cli#303 (#282) → this → cli#284 → cli#285. Branch is based on
refactor/282-data-go-file-split; until #303 merges, this diff shows its commit too. Review only the top commit (a2464db).🤖 Generated with Claude Code
Note
Low Risk
Mechanical move-only refactor with explicit parity claims and unchanged tests; risk is limited to subtle wiring mistakes (named returns, defer, cancelled/overwrite), not new product behavior.
Overview
Splits the oversized
runDataIngestflow into two named helpers sodata.gostays under the line budget, with no intended behavior change.resolveLocalInput(data_ingest_local.go) now owns all pre-cluster work: guards, banner, interactive prompts, validation, layout discovery, spec build/schema check, local preflight, and summary. It still mutates*runDataIngestArgsand returnscancelledfor clean interactive aborts.connectIngestTarget(data_ingest_cluster.go) owns kube discovery, verbose cluster summary, and the existing-table guard (including settinga.Overwritefrom the replace prompt). It returnscancelledwhen the user declines replace.runDataIngestkeeps the--output-jsonerror defer, dry-run, staging, andrunIngestionRunorchestration; it only calls the two helpers and handles theircancelledpaths.Reviewed by Cursor Bugbot for commit a2464db. Bugbot is set up for automated code reviews on this repo. Configure here.