Skip to content

feature: Phase 0 Java foundation for the COBOL translation (copybook models, subroutine contracts, test harness) - #256

Open
devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
devin/1785227633-phase0-java-foundation
Open

devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
devin/1785227633-phase0-java-foundation

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 28, 2026

Copy link
Copy Markdown

Summary

Phase 0 of the COBOL-to-Java translation: the shared foundation every later slice (PORTTRAN, portfolio CRUD, batch, reporting) builds on. Nothing from src/programs/ is translated yet — this is the five shared copybooks, the two subroutine contracts PORTTRAN calls, and the harness that stands in for the z/OS runtime the original was never run on.

New tree under modernized/java/ (67 tests, mvn -f modernized/java/pom.xml test):

model/    TRNREC -> TransactionRecord   POSREC -> PositionRecord   PORTFLIO -> PortfolioRecord
          ERRHAND -> ErrorMessage + ErrorCategory/ErrorSeverity/VsamStatus
          AUDITLOG -> AuditRecord + AuditType/AuditAction/AuditStatus
          level-88 sets -> enums; CobolDecimal / CobolText carry the storage semantics
service/  AuditProcessor (CALL 'AUDPROC')   ErrorProcessor (CALL 'ERRPROC')
src/test/ TestData seeded from the docs, Recording{Audit,Error}Processor doubles, DocumentedRulesTest oracle
          CopybookFidelityTest pins every width, scale, level-88 set and setter-overload pair against the .cpy files

Three decisions drive everything else:

Packed decimals are BigDecimal at the picture's scale, truncated not rounded. No statement in the slice says ROUNDED or ON SIZE ERROR, so storage drops excess decimals toward zero and silently loses high-order digits:

setTrnAmount("11234567890123.459")  // PIC S9(13)V9(2) -> 1234567890123.45
setTrnQuantity("0.99999")           // PIC S9(11)V9(4) -> 0.9999

Coded fields keep their raw bytes next to the enum. 2120-CHECK-TRANSACTION-TYPE exists because TRN-TYPE can hold something no level-88 covers, and its message echoes it — so getTrnType() returns "XX" while getTransactionType() returns null. Same split for every other coded field.

PIC X(n) is stored space-padded. ERR-TEXT is PORTTRAN's error flag (IF ERR-TEXT = SPACES), not just a message, so it is an 80-char buffer with isErrTextSpaces() / clearErrText(). CobolText.isSpaces compares against the space character only, since COBOL tests X'40' and a tab is data.

Discrepancies handled explicitly, not fixed

All are written up as G1G10 in modernized/java/TRANSLATION-NOTES.md, each with a test:

  • G1 — COPY PORTREC names a copybook that does not exist, and PORTTRAN updates PORT-TOTAL-UNITS/PORT-TOTAL-COST, which PORTFLIO.cpy does not define (it has PORT-TOTAL-VALUE/PORT-CASH-BALANCE). PortfolioRecord translates PORTFLIO faithfully and adds two clearly-marked synthetic fields typed from their nearest documented equivalents in POSREC.cpy: portTotalUnits as POS-QUANTITY S9(11)V9(4), portTotalCost as POS-COST-BASIS S9(13)V9(2). They are excluded from toRecordImage() so the audit before-image stays a faithful PORTFLIO picture. Reusing PORT-TOTAL-VALUE for cost was rejected — market value and cost basis are different quantities.
  • G2 / G3 — the dead 2200-UPDATE-POSITIONS subtree and the unimplemented transfer are documented here and are Child 1's to reproduce.
  • G4 — neither subroutine's linkage area matches what the caller passes. AUDPROC expects LS-AUDIT-REQUEST (starts at LS-SYSTEM-INFO, ends with a return code) but is handed AUDIT-RECORD (starts with a 26-byte timestamp, no return code); ERRPROC is offset by ERR-MESSAGE's 18-byte timestamp. The interfaces pass the typed record and return the status instead of reproducing a storage overlay that would corrupt data. Also: PORTTRAN tests RETURN-CODE, which AUDPROC never sets.
  • G5 — the documentation describes a different system than the copybooks (account/fund vs portfolio/investment). Rule: copybooks win for anything a program reads or writes, docs win for validation rules, ranges and the error catalogue. The three specific conflicts — 9-char PORT00001 into PIC X(8), documented types B/S vs BU/SL/TR/FE, documented status I vs level-88 A/C/S — are each pinned by a test in DocumentedRulesTest.
  • G6PORTTRAN never sets ERR-CODE/ERR-SEVERITY, so ERRPROC reports severity 0 ("success") for every error. Reproduced; the documented catalogue lives separately as ErrorCode for the oracle.
  • G7 / G8 — audit SUCC/FAIL is decided by a possibly-stale WS-PORT-STATUS, and 2300-UPDATE-AUDIT-TRAIL STRINGs COMP-3 senders, which Enterprise COBOL rejects — more evidence the source was never compiled. Note that GnuCOBOL accepts the latter even under -std=ibm, so cobc cannot corroborate G8; it is verified against the Enterprise COBOL V6.4 language reference instead.
  • G9 / G10 — found by compiling the source rather than grepping it: the portfolio record area is REWRITE PORTFOLIO-RECORD in three paragraphs but MOVE PORT-RECORD in a fourth (so one of the two names is undefined — a second symptom of G1), and MOVE FUNCTION USER-ID TO AUD-USER-ID uses an intrinsic Enterprise COBOL does not have, leaving the audit user with no defined source.

Section 7 of the notes is the contract for the parallel slices: consume these types, never redefine them, one file set per slice, methods named after paragraphs, error strings byte-for-byte.

Review pass

An adversarial verification pass over the first commit found the models field-by-field correct against the copybooks, and one real bug: CobolDecimal's String factories did new BigDecimal(value) while the BigDecimal ones mapped null to zero, so ten setters across all five models threw or didn't purely by the argument's static type. Fixed, along with isSpaces treating tabs as blank, and CopybookFidelityTest is now in the suite so a later slice that widens a field or relaxes a scale fails immediately. Details in the comment below.

Link to Devin session: https://app.devin.ai/sessions/c6183caec9fa435aaa59c67158c189f8
Requested by: @gaelkekatos-jpg


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)

…pybook models, subroutine contracts, test harness)

Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

…st; document G9/G10

Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
@devin-ai-integration

devin-ai-integration Bot commented Jul 28, 2026

Copy link
Copy Markdown
Author

Phase 0 — independent test results, and three fixes on top

Verified on JDK 11 / Maven 3.6.3 / GnuCOBOL 3.1.2: clean-state build plus an adversarial review of the translation against the COBOL source. The copybook translation came back field-by-field correct — every width, scale, group length and level-88 set matches the .cpy files. Three things did not, and are fixed in 5686639.

Fixed: String-overload packed setters threw NPE on null

CobolDecimal.store() maps null to zero and every BigDecimal overload honoured that; the String factories bypassed it, so a reflection sweep of every public single-arg setter across all five models found 56 null-tolerant and 10 that threwsetTrnQuantity/Price/Amount, setPosQuantity/CostBasis/MarketValue, setPortTotalValue/CashBalance/TotalUnits/TotalCost. The asymmetry was invisible at the call site: setTrnQuantity(null) was safe or fatal purely by the argument's static type, which is exactly the kind of trap four downstream slices would have hit.

-    return quantity(new BigDecimal(value));          // NPE when value == null
+    return quantity(value == null ? null : new BigDecimal(value));

Storage probe: 10 String-overload setters threw NPE on null

Fixed: isSpaces treated tabs and low values as blank

It used String.trim(), so "\t" and "\0" reported as SPACES. COBOL compares against X'40' only. This one matters because IF ERR-TEXT = SPACES is PORTTRAN's error flag — the whole validation chain short-circuits on it.

Fixed: two source discrepancies missing from the notes (now G9, G10)

Compiling rather than only grepping surfaced two the original write-up missed:

  • G9 — the portfolio record area is REWRITE PORTFOLIO-RECORD at PORTTRAN.cbl:194,219,242 but MOVE PORT-RECORD at :278. Whichever copybook was intended, one of the two names is undefined — a second, independent symptom of the missing PORTREC.cpy.
  • G10MOVE FUNCTION USER-ID TO AUD-USER-ID at :254 uses an intrinsic that does not exist in Enterprise COBOL (the neighbouring FUNCTION CURRENT-DATE is real), so AUD-USER-ID has no defined source.

Also promoted the reviewer's CopybookFidelityTest into the suite as a permanent regression guard: it reads its expected widths and scales from the copybooks rather than the Java, so a later slice widening a field or relaxing a scale fails immediately. 58 → 66 tests, all green.

✅ Build reproducibility and the non-standard source layout

Clean build, three independent test counts

Test count corroborated three ways (surefire XML attributes, @Test count, <testcase> count). No @Disabled or assumeTrue; skipped=0 on every report. DocumentedRulesTest reporting 0 is legitimate — its tests live in four @Nested classes.

Both source roots proven load-bearing by negative control: removing build-helper-maven-plugin drops 21 sources to 19 and fails with package com.clbs.portfolio.service does not exist.

Source roots, positive and negative control

✅ Copybook fidelity and storage semantics — no defects found

Probes drove every PIC X(n) field with a 400-character marker and every packed field past capacity, with expected values read from the .cpy files. Every field matched: all widths, TRN-QUANTITY/TRN-PRICE scale 4 over 11 integer digits, TRN-AMOUNT scale 2 over 13, and all group lengths (TRN-KEY 28, POS-KEY 26, PORT-KEY 18, ERR-TIMESTAMP 18, AUD-HEADER 58, AUD-KEY-INFO 18). All 12 level-88 sets exact, including the space-padded AUD-ACTION values; cross-set codes correctly return null.

Storage held under adversarial input: truncation toward zero for negatives (-1.005-1.00), high-order wrap on both signs, exact capacity preserved and one unit past it wrapping, picX padding and truncating on the right. No double or float anywhere.

✅ G1–G8 all verified true of the source, with one caveat on G8

G1 (missing PORTREC.cpy, confirmed empirically with cobc), G2 (2200-UPDATE-POSITIONS defined at :167, never performed), G3, G4 (linkage mismatches at AUDPROC.cbl:34-49 / ERRPROC.cbl:38-45), G5, G6, G7, G8.

G1 confirmed with cobc

Caveat on G8: GnuCOBOL 3.1.2 accepts STRING over COMP-3 in both its default dialect and -std=ibm, so the local compiler does not corroborate it. Verified instead against the Enterprise COBOL V6.4 STRING rules — senders must be usage DISPLAY/DISPLAY-1/NATIONAL/UTF-8, and a numeric sender must be an integer — invalid on two independent grounds. This caveat is now recorded in the notes, because cobc is a weak oracle for IBM-dialect claims generally.

Known, deliberate, non-blocking
  • PortfolioRecord.toRecordImage() renders 164 characters against the copybook's 148 bytes, because each 8-byte packed field renders as a 16-character image. It is a rendering for group moves, not a byte-accurate record layout; section 3.3 of the notes now says so explicitly and CopybookFidelityTest pins the length so nobody starts relying on it.
  • setErrSeverity(int) doesn't enforce PIC S9(4) COMP capacity. The only values any program assigns are 0/4/8/12/16, so left as is.
  • The COBOL cannot be executed (PORTTRAN.cbl doesn't compile, by G1), so no differential testing against a running program was possible. The documented rules remain the oracle.

Re-verification of the fixes (5686639), and one more found

The same sweeps were re-run against the fixes, plus mutation testing of the newly-committed CopybookFidelityTest. Both fixes hold: 66 setters swept across both overloads, 0 throw (previously exactly 10), null zeros land at the picture's scale rather than BigDecimal.ZERO scale 0, and the fix stayed narrow — "not-a-number", "1,234.00", "" and "abc" still throw NumberFormatException, so null-safety did not become silent-zero-for-garbage. isSpaces is now false for \t, \n, \r, \0, \u000B, \u001F, \u00A0 and " \t ", still true for real space-padded defaults. G9, G10, the G8 caveat and the 148-byte arithmetic in §3.3 all check out against the source.

D1 and D3a fixes verified

The regression guard was itself only half a guard

Mutation testing is the interesting part. Three of four mutations failed with the predicted assertion message — a width constant, a String-overload scale, a level-88 literal. The fourth did not: changing PositionRecord's BigDecimal overload of setPosQuantity from quantity to amount, a silent scale 4→2 error, left the whole suite green. Sweeping all ten numeric setters, 5 of 10 BigDecimal-overload scale mutations survived; the other five were caught only incidentally by other tests, not by the fidelity test.

5 of 10 BigDecimal-overload mutations survived

The cause is the same shape as D1 itself: every numeric field was driven through its String setter only, so the other overload was unguarded. Since this test is now permanent and four child slices are told to extend it, that blind spot would have been inherited. Fixed in 813be0b by storing each field both ways and comparing — BigDecimal.equals compares scale, so a picture applied to one overload and not the other fails whether it differs in scale or capacity:

type.getMethod("set" + field, String.class).invoke(viaString, "123456789012345.98765");
type.getMethod("set" + field, BigDecimal.class).invoke(viaDecimal, new BigDecimal(sample));
assertEquals(getter.invoke(viaString), getter.invoke(viaDecimal));

Re-running the surviving mutation against it now fails with PosQuantity: the String and BigDecimal overloads must store the same field ==> expected: <56789012345.9876> but was: <3456789012345.98>. 67 tests green.

Also recorded, per the review: ERR-SEVERITY deliberately does not clamp to PIC S9(4) COMP. Every other numeric field clamps to its picture, so §3.4 now says outright that this one doesn't and why (only 0/4/8/12/16 are ever assigned), rather than leaving a downstream slice to assume it does.

Mutation results

…est; note ERR-SEVERITY is unclamped

Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
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.

0 participants