fix(cli): validate CSV headers, encoding and numeric bounds

[cristim]

CSV input with missing headers now fails before row processing instead of loading incomplete recommendations. UTF-8 BOM input preserves Service values, invalid UTF-8 bytes are rejected, Count must be at least 1, and EstimatedSavings must be nonnegative. Errors include the CSV position without repeating invalid numeric values.

Refs #1327
Closes #2116

The two issues share the parser and retain separate commits. Required columns currently stay Service, Region, ResourceType and Count, preserving existing minimal-CSV inputs. The owner must decide whether the Term and PaymentOption headers proposed in #1327 should become required. Until that decision is settled, #1327 remains open. Their value validation remains tracked by #379.

Verification at d75df92, based on 65a02a7:

• Behavioral failing-first proof: current regression tests with parent parser ad57326 fail for missing headers, BOM Service fidelity, Count zero and negative savings. Encoding tests with pre-guard parser3111dc7d fail for Latin-1 fields and malformed TOTAL rows. Current tests pass.
• Full GOWORK=off GOTOOLCHAIN=go1.26.6 go test -short -race -count=1 ./...: passed530.548s. No tests in cmd branch on testing.Short.
• Fresh build, pinned golangci-lint2.10.1 (zero issues), repository gosec and all commit hooks pass.
• Real macOS CLI dry runs with isolated credential-free environment reject missing legacy headers, zero Count, negative savings and Latin-1 input with exit1 before cloud configuration. No reports or purchase records; startup initializes empty audit logs. These scenarios use temporary CSV fixtures.
• Independent local review: two passes, findings fixed, no actionables. Rebase range-diff preserves all three patches.

Merge gate remains pending: project requires independent exact claude-opus-5-5 review naming the final SHA. Historical Kimi and current local Codex reviews do not satisfy that model gate; local exact-model runtime reports Not logged in. No merge authorization is inferred from CI or fixture verification.

Follow-up issues filed: none. Existing #379 tracks the unchanged Term and PaymentOption validation gap.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 18c426a9-5ed8-42f2-998b-326fa9713224

📥 Commits

Reviewing files that changed from the base of the PR and between 65a02a7 and d75df92.

📒 Files selected for processing (4)

• cmd/multi_service_csv.go
• cmd/multi_service_csv_header_test.go
• cmd/multi_service_csv_strict_test.go
• cmd/multi_service_csv_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The CSV loader now rejects missing required headers, invalid UTF-8, counts below one, and negative savings. It strips a UTF-8 BOM from the first header and reconstructs service-specific Details from non-empty Engine and Deployment fields.

Changes

CSV input validation

Layer / File(s)	Summary
Header and record validation cmd/multi_service_csv.go, cmd/multi_service_csv_header_test.go	The loader checks for Service, Region, ResourceType, and Count headers, validates UTF-8 in headers and records, and strips a BOM from the first header. Tests cover missing and legacy headers, encodings, quoted fields, line endings, and other record cases.
Count and savings validation cmd/multi_service_csv.go, cmd/multi_service_csv_strict_test.go, cmd/multi_service_csv_test.go	Counts below one and negative savings now produce parsing errors. Error messages omit the invalid input value. Tests confirm that zero savings remain valid.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d75df

No actionable merge-blocking issue is established by the supplied evidence. Normal checks and the stated independent review requirement still apply.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	#2116 is implemented: Count rejects zero and negative values, EstimatedSavings rejects negative and non-finite values, and parse errors include the CSV position without an outer duplicate value. #1327…	Update the required CSV header set to include Term and PaymentOption, then add or update missing-header tests for both columns. Preserve the existing edge-case and numeric-bound tests.
Out of Scope Changes check	⚠️ Warning	The PR summary reports, and the reviewed parser implements, reconstruction of service-specific Recommendation.Details from Engine and Deployment fields. This changes valid CSV parsing and purchase-pat…	Remove the Engine/Deployment-to-Details reconstruction from this pull request, or link a directly applicable issue that requires this behavior and limit the change to that documented requirement.
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: CSV header validation, encoding validation, and numeric bounds validation.

Full details: Linked Issues check

Explanation

#2116 is implemented: Count rejects zero and negative values, EstimatedSavings rejects negative and non-finite values, and parse errors include the CSV position without an outer duplicate value. #1327 is mostly implemented: the loader validates Service, Region, ResourceType, and Count before row processing, strips a UTF-8 BOM, rejects invalid UTF-8, propagates field-count errors, skips TOTAL rows, and adds tests for the listed CSV edge cases. However, #1327 specifies Term and PaymentOption in the required header set. The reviewed code keeps both columns optional. The linked issue therefore remains partially unmet.

Full details: Out of Scope Changes check

Explanation

The PR summary reports, and the reviewed parser implements, reconstruction of service-specific Recommendation.Details from Engine and Deployment fields. This changes valid CSV parsing and purchase-path behavior, but #1327 and #2116 require header, encoding, row, and numeric validation. The linked issues do not request this reconstruction.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Fresh baseline verification at main65a02a72 also passed: full pinned short race suite532.024s and fresh build. PR head d75df92 full race suite530.548s, pinned lint0issues, fresh build and malformed-input CLI checks pass. CI tests/lint/build/pre-commit passed; failed steps are only gosec/Trivy SARIF uploads, with no error annotations. Rerunning the failed security job and CI summary to isolate an upload/service failure; no checks bypassed. Exact claude-opus-5-5 final-SHA review still blocks merge.

[cristim]

CodeRabbit review at d75df92 reports no actionable comments and no inline findings. Pre-merge warnings triaged: (1) required-column contract is an owner decision because issue #1327 proposes six headers while existing minimal CSV inputs require four. Changed the PR to Refs #1327 so it stays open pending that decision; #379 separately tracks value validation. (2) Engine/Deployment Details reconstruction is already present on base65a02a72 and absent from this PR diff, so removing it would be an unrelated regression. (3) Boilerplate docstrings conflict with the project comment policy; local review removed branch-added narration rather than adding it. Security-upload retry completed successfully on attempt2; CI is green. Exact claude-opus-5-5 final-SHA review remains required before merge.