feature: translate PORTTRAN.cbl to PortfolioTransactionProcessor (Child 1) - #257
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Child 1 of the COBOL-to-Java translation, stacked on the Phase 0 foundation. Translates src/programs/portfolio/PORTTRAN.cbl paragraph by paragraph, preserving its defects rather than repairing them: the main flow only validates because the whole position-update subtree is unreachable (G2), transfers stay unimplemented (G3), errors carry no code or severity (G6) and the audit status is decided by a possibly stale portfolio file status (G7). Adds PortfolioRepository and TransactionSource for the two files, 47 tests, and six new discrepancy entries (G9-G14) in TRANSLATION-NOTES.md. Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Phase 0 catalogued the duplicate record-area names and FUNCTION USER-ID as its own G9 and G10 while this branch was open. Keeps the Phase 0 wording for both and folds Child 1's pinning tests into them; Child 1's remaining entries stay G11-G14. Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
| `MOVE` pads to the 26 bytes of `AUD-TIMESTAMP`. Pinned by `ControlFlow.userIdIsSupplied` and | ||
| `AuditTrail.successfulUpdateIsAuditedSucc`. | ||
|
|
||
| ### G11 - an unrecognised transaction type is audited as if something had happened |
There was a problem hiding this comment.
📝 Info: Section 4 intro still says discrepancies are catalogued G1-G10 after G11-G14 were added
This PR appends four new discrepancy entries (G11-G14) to section 4 of modernized/java/TRANSLATION-NOTES.md (added at lines 285-329), but the summary sentence at modernized/java/TRANSLATION-NOTES.md:15 still reads "Discrepancies found so far are catalogued as G1-G10 in section 4." That line is now stale; it should read G1-G14. Minor documentation inconsistency only (the line itself is unchanged context, not part of an edited hunk).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in 3257f93: the header line now reads G1-G14.
| public void checkAmounts() { | ||
| if (CobolDecimal.isNotPositive(transactionRecord.getTrnQuantity())) { | ||
| errorMessage.setErrText(ERR_QUANTITY_NOT_POSITIVE); | ||
| return; | ||
| } | ||
|
|
||
| boolean transfer = "TR".equals(transactionRecord.getTrnType()); | ||
|
|
||
| if (CobolDecimal.isNotPositive(transactionRecord.getTrnPrice()) && !transfer) { | ||
| errorMessage.setErrText(ERR_PRICE_NOT_POSITIVE); | ||
| return; | ||
| } | ||
|
|
||
| if (CobolDecimal.isNotPositive(transactionRecord.getTrnAmount()) && !transfer) { | ||
| errorMessage.setErrText(ERR_AMOUNT_NOT_POSITIVE); | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: checkAmounts matches the COBOL ELSE-IF (EXIT PARAGRAPH) short-circuit ordering
I verified checkAmounts() against 2130-CHECK-AMOUNTS in src/programs/portfolio/PORTTRAN.cbl. The COBOL uses EXIT PARAGRAPH after the quantity and price checks and applies AND TRN-TYPE NOT = 'TR' only to the price and amount checks. The Java return after quantity and after price reproduces the first-failure-wins ordering exactly, and the !transfer guard on price/amount (lines 328-334) matches. A non-transfer with quantity>0, price<=0 and amount<=0 reports the price error in both, so no ordering divergence exists. Not a bug — faithful reproduction.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Matches my reading of 2130-CHECK-AMOUNTS — the quantity check applies to every type and only price/amount carry AND TRN-TYPE NOT = 'TR', with first failure winning. Deliberate, no change.
| private String currentDate() { | ||
| ZonedDateTime now = ZonedDateTime.now(clock); | ||
| int offsetMinutes = now.getOffset().getTotalSeconds() / 60; | ||
| char sign = offsetMinutes < 0 ? '-' : '+'; | ||
| int absoluteMinutes = Math.abs(offsetMinutes); | ||
| return String.format( | ||
| "%04d%02d%02d%02d%02d%02d%02d%s%02d%02d", | ||
| now.getYear(), | ||
| now.getMonthValue(), | ||
| now.getDayOfMonth(), | ||
| now.getHour(), | ||
| now.getMinute(), | ||
| now.getSecond(), | ||
| now.getNano() / 10_000_000, | ||
| sign, | ||
| absoluteMinutes / 60, | ||
| absoluteMinutes % 60); | ||
| } |
There was a problem hiding this comment.
📝 Info: currentDate renders the 21-char CURRENT-DATE format and MOVE-pads to 26
currentDate() builds YYYYMMDDhhmmssnn±hhmm (21 chars): now.getNano() / 10_000_000 yields hundredths of a second, and the GMT offset is rendered as signed hhmm. This is 21 characters which the audit setter pads to the 26-byte AUD-TIMESTAMP, matching the test constant 2024032015304512+0000. The %s format specifier receiving a char sign autoboxes to Character and renders correctly. Faithful to MOVE FUNCTION CURRENT-DATE TO AUD-TIMESTAMP.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed, and that is the intent — FUNCTION CURRENT-DATE returns 21 characters and MOVE pads them into the 26 bytes of AUD-TIMESTAMP. No change.
Co-Authored-By: Gael Kekatos <gael.kekatos@cognition.ai>
Summary
Stacked on PR #256 — this targets
devin/1785227633-phase0-java-foundation, notmain, because it consumes the Phase 0 models, subroutine contracts and test harness. Rebase ontomainonce Phase 0 lands.Child 1 of the coordinated COBOL-to-Java translation:
src/programs/portfolio/PORTTRAN.cblbecomesmodernized/java/service/PortfolioTransactionProcessor.java, one method per paragraph, with the paragraph named in each method's Javadoc.mvn -f modernized/java/pom.xml testis green: 113 tests, 47 of them new.The most important behaviour preserved is that the main flow does nothing but validate. Nothing in the COBOL performs
2200-UPDATE-POSITIONS, so the entire position-update and audit subtree is unreachable (G2). That is reproduced rather than repaired:so
main()over a stream of valid buys leaves the repository untouched and never callsAUDPROC— pinned byDeadCode.validBuyChangesNothing. The update paragraphs are translated anyway as public methods nothing calls, so the logic is captured and directly testable.Other defects reproduced, not fixed: transfers stay unimplemented (G3),
9000-ERROR-ROUTINEleavesERR-CODEblank andERR-SEVERITYzero (G6), and2300-UPDATE-AUDIT-TRAILdecidesSUCC/FAILfromWS-PORT-STATUS, which on a path that did no I/O is whatever the last file operation left (G7).Interfaces added (
modernized/java/service/)FD TRANSACTION-FILE, sequentialTransactionSource—open(),read()returningnullforAT END,close()FD PORTFOLIO-FILE, indexed I-OPortfolioRepository—open(),findById()returningOptional.empty()forINVALID KEY,update(),close(),getFileStatus()getFileStatus()exists because2300branches onWS-PORT-STATUS(G7). Repository, source,AuditProcessorandErrorProcessorare constructor-injected; the class does no I/O, holds no static state, and every decimal is aBigDecimalat its picture's scale.Clockand the user id are also injectable, soFUNCTION CURRENT-DATEis deterministic in tests.Rendering decisions worth checking
STRING ... DELIMITED BY SIZE INTOoverlays the receiver from the left, does not pad, and has noON OVERFLOWhere, so an id ofPORT99yieldsInvalid Portfolio ID: PORT99— the sending field's trailing spaces included.2130-CHECK-AMOUNTSapplies the quantity check to every type but writes the price and amount checksAND TRN-TYPE NOT = 'TR', so a transfer validates with a zero price and zero amount. Left exactly as written.AUD-MESSAGEstrings twoCOMP-3fields, which is not legal COBOL (G8). The packed senders are rendered asCobolDecimal.imagerenders them, givingTransaction: BU Amount: +000000001250000 Units: +000000001000000for the seeded buy.New discrepancies (
TRANSLATION-NOTES.mdsection 4, each with a test)EVALUATEin2200has noWHEN OTHER, so an unrecognised type updates nothing and is still audited, with a blank action.AUD-BEFORE-IMAGEis captured after the update despite the comment saying otherwise, andAUD-AFTER-IMAGEis never populated.OPEN I-O PORTFOLIO-FILEis logged and then ignored, since onlyWS-TRAN-STATUSgates the loop;3000-TERMINATEcloses both files regardless.Phase 0 catalogued G9 (the record area is
PORTFOLIO-RECORDin one paragraph andPORT-RECORDin another) and G10 (FUNCTION USER-IDis not an intrinsic) while this branch was open; the merge keeps its wording and folds this slice's pinning tests and rendering decision into those entries.TRANSLATION-NOTES.mdalso flips the Child 1 row to landed and gains aPORTTRANsection with the full paragraph-to-method, file-to-interface and error-string mapping. No Phase 0 type was changed.Link to Devin session: https://app.devin.ai/sessions/0b3ee627d5e242fdb56ca5519b7c147d
Requested by: @gaelkekatos-jpg
Devin Review