Skip to content

fix(etl): reject invalid amounts before persistence - #199

Draft
seonghobae wants to merge 11 commits into
developfrom
fix/reject-invalid-amount-622e5e6
Draft

fix(etl): reject invalid amounts before persistence#199
seonghobae wants to merge 11 commits into
developfrom
fix/reject-invalid-amount-622e5e6

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Fix #198 test-first from protected develop@622e5e6c3d534f230c390f10e3832efadfc01825. The protected ETL transformation historically mapped malformed, blank, excessive-precision, and unsupported-scale AMOUNT input to valid-looking 0.00, making bad upstream data indistinguishable from a genuine zero after persistence.

This direct-develop Draft now contains the complete bounded RED → GREEN behavior repair plus source-backed documentation alignment. It does not add a database migration, new public error field, raw-value logging, dependency change, or broad ETL refactor.

Exact current identity

  • protected/live base: develop@622e5e6c3d534f230c390f10e3832efadfc01825;
  • branch: fix/reject-invalid-amount-622e5e6;
  • fail-first source head: 1b89e62d5f8b30c6dceb0d39f538d283238e7a8a;
  • exact current source head: 27707b72ba1865fe86e24dfbecdc740a586a258b;
  • current synthetic merge preview: f2d5e2042873b31047afda21b7bfb385e9794c4b;
  • Draft: true;
  • GitHub mergeability: true;
  • formal reviews: none;
  • unresolved review threads: zero.

Every check/review from a predecessor head or base is historical and does not transfer.

RED — invalid amounts were accepted and rewritten

Fail-first 1b89e62d5f8b30c6dceb0d39f538d283238e7a8a added EtlServiceAmountIntegrityTest before production changed. It reaches EtlService.processData(...) with malformed, blank, excessive-precision and unsupported-scale values and requires EtlRequestError.INVALID_RECORD before JDBC; it also requires one bad amount in a multi-record request to reject the entire prevalidated batch before the first write.

Hosted CI 31348838751, macOS job 93335761349, checked out synthetic merge e292fcd38b3f6af5d2efe9b332cc65ed1fdc8d0a (Merge 1b89e62... into 622e5e6...). Production/test compilation succeeded. EtlServiceAmountIntegrityTest then ran six tests and all six failed because production threw no EtlRequestException; the rest of the ETL suite remained green before Maven stopped. This is valid RED at the real transformation/admission boundary, not a setup/import/fixture failure.

GREEN — fail closed before persistence

The bounded production repair changes the amount transformation so malformed/blank/unsupported numeric input is classified through the existing stable invalid-record path rather than rewritten to zero. Valid values retain deterministic BigDecimal formatting and HALF_UP scale-2 behavior. Whole-batch prevalidation, transaction boundaries, idempotency, payload limits, response shape and JDBC parameterization remain unchanged.

Subsequent tests preserve genuine zero separately from invalid input and cover precision/scale boundaries and whole-batch rejection before JDBC.

Documentation RED → GREEN

After behavior was green, EtlBatchDocsAlignmentTest required the changelog to state the buyer-visible fail-closed amount-integrity contract. CI 31358691197, macOS job 93363094463, reached that documentation boundary with EtlServiceAmountIntegrityTest 6/6 green and failed only changelogRecordsFailClosedAmountIntegrity because the existing sentence did not contain the canonical lowercase phrase required by the contract.

Exact current head 27707b72ba1865fe86e24dfbecdc740a586a258b changes only that changelog wording: invalid AMOUNT values fail closed before persistence instead of being rewritten to 0.00, preserving the distinction from a genuine zero.

Current hosted proof

All exposed workflow aggregates for exact source head 27707b72ba1865fe86e24dfbecdc740a586a258b are terminal-success:

  • CI 31361048737: success on macOS, Ubuntu and Windows;
  • Dependency Review 31361048765: success;
  • CycloneDX SBOM 31361048779: success;
  • SAST Semgrep 31361048735: success;
  • Security Scan 31361048747: success.

CI macOS job 93369857424 checked out synthetic merge f2d5e2042873b31047afda21b7bfb385e9794c4b, not literal source head. On that integration tree EtlBatchDocsAlignmentTest passed 5/5, EtlServiceAmountIntegrityTest passed 6/6, ETL ran 283 tests with zero failures/errors/skips, CDC ran 106 tests, gateway tests passed, and the reactor finished BUILD SUCCESS.

The same CI still reproduces the inherited protected JaCoCo false-green: Analyzed bundle 'etl-service' with 0 classes followed by coverage success. #162/#164 owns selected-bundle non-vacuity, and #205 owns repository-wide coverage scope. Current protected PR workflows also use synthetic merge checkout rather than accepted literal source. #196 separately owns complete Maven dependency-resolution evidence for vulnerability scanning. These controls are independent of this amount-integrity fix.

Merge boundary

Keep Draft. Merge only when the unchanged exact source head has accepted literal-source deterministic/security evidence, complete same-revision dependency/vulnerability evidence, non-vacuous owned-production coverage, every required repository/security gate, zero valid unresolved review findings, and qualifying independent non-author formal approval where governance requires it. No predecessor-head, other-PR, status-only, skipped-required, incomplete-scanner, or synthetic-merge-only evidence transfers.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1366978f-043a-47f2-8084-b437d65a2f2a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Data integrity] Reject invalid AMOUNT values instead of silently coercing them to zero

1 participant