feat: file each commodity row in its Schedule B unit, and record Incoterms on the CEVA SLI - #50
Merged
Merged
Conversation
…terms on the CEVA SLI Two problems that both end in a form stating something the shipment does not. **Unit of quantity.** Several Schedule B numbers are reported in kilograms while every invoice in this trade counts pieces — `9031.90.0000` is the one on the sample shipments — and the app has always warned about it while filing the piece count anyway. Each row is now filed in the unit its commodity number requires, taking the net weight where the code is reported by weight, and the unit is changeable per commodity number on the review screen for the codes that accept more than one (`NO+KG`), where only the filer knows how the goods are measured. Resolved once in `reconcile` and read from there by both SLI adapters and both keying sheets, so the paperwork prepared for one shipment cannot disagree with itself. The document's own figures are never overwritten — `quantity` and `sourceUom` still say what was printed, and the totals still reconcile against them. Where nothing supports the required unit (a layout that prints no weights), the printed figure is filed and the reconciliation says why; no quantity is invented to fit a unit. Units a shipment cannot state are offered and disabled with the reason rather than hidden. **Incoterms on the CEVA form.** The box was ticked by matching the raw string against the box names, so `DAP Singapore` from the in-house invoice form and `FOB Origin - Collect` from the Vendor A trade terms ticked nothing at all — a form filed with no delivery term on it. The rule is now read off the string (bare code, named place, lower case, or written out in words) and both adapters use the same reader. The named place is kept: it goes to Nippon's box 15, and to CEVA's special instructions, which is the only place that form can record it. A rule withdrawn since Incoterms 2020 is reported with its replacement rather than silently remapped — reclassifying a delivery term is the filer's decision. The filed vendorA3 expectation for box 24 changes with this: its third row carries 4.263 KG in place of the 12 pieces that were filed against a code reported by weight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013x3f6KRnFFe1R83BSTZewU
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Status
FindingsNo issues found in changed scope. Proposed patchesNone CI failure mappingCI is still running (pending). Diagnostic plannpm ci
npm run lint
npm test -- --watch=false
npm run build
npx --yes markdown-link-check <changed-docs>
npx --yes cspell <changed-docs>Next actions
MetaProcessed at 2026-08-20 17:14:30.702 UTC |
…ling, unit spellings Ten findings from the review on #50, all in the new unit-of-quantity and Incoterm paths. **Correctness** - Restated quantities were rounded to three decimals whatever the conversion, so every down-scaling unit lost the figure it was converting. 4.263 kg filed as tonnes came out `0.004` — 6% of the shipment's declared weight gone into the rounding. The places now scale with the conversion: dividing by a thousand buys three more of them. - A within-family conversion reported `basis: 'source'`, so the review screen said "As invoiced" and the Schedule B check said "matching the invoice" about a figure the document does not carry. New `converted` basis, explained wherever a filed quantity is. - `scheduleBUnit` was canonicalised, which filed `NO` against the 51 commodity numbers the Census file reports in `PCS` — a unit it does not list for them. Canonical forms decide what converts into what; the filed unit keeps the spelling it was asked for. - The keying sheet had no case for the eight codes Schedule B files with no quantity: it keyed a literal `0` with no note. It now keys the document's own count and says the figure is for the application rather than the declaration. - The Commodities grid still summed the quantity column across rows that can now be in different units, printing pieces added to kilograms. Blank when mixed, as the review screen already does for the same column. - `parseIncoterm`'s named place is whatever follows the rule, and `FOB Origin - Collect` put "Origin - Collect" into a box captioned "NAMED PLACE/PORT". A freight-qualifier guard keeps payment terms out of it. - CEVA preferred the document's named place, Nippon the operator's — the same two inputs filed two ways, and CEVA discarded the operator's entry silently. Both now take the operator's, and CEVA warns when it overrides a place the document states. - `basisNote` said "by your choice" whenever a choice existed for the code, including on a row where the choice was unreachable and discarded. It now reports whether the choice was honoured, so that row points at its missing weight instead. **Structure** - `parseIncoterm` moves to `src/domain/incoterms.ts`. It is domain knowledge, not carrier knowledge, and `incotermFromTradeTerms` was carrying a second unvalidated copy that accepted any leading three letters — `PPD Collect` yielded an Incoterm of `PPD`. - `roundTo` had been duplicated into `units.ts`; `reconcile/lines.ts` now re-exports the one definition. - Dropped a redundant guard and folded an unreachable arm in the CEVA Incoterm branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013x3f6KRnFFe1R83BSTZewU
JoelA510
pushed a commit
that referenced
this pull request
Aug 20, 2026
A follow-up review of #50 found thirteen issues. Two of them defeated the precision fix that PR shipped, which is the reason for this one. **The quantity never reached the form at full precision.** `roundScaled` chose a precision to suit each conversion, and then both adapters rounded to exactly three decimals on the way into the box. A commodity number reported in tonnes — 247 of them in the shipped Census file — printed `0.004` against a 4.263 kg row, under-declaring by 6%, and a 0.4 kg row printed `0.000`. The keying sheet kept the full figure, so the SLI and the sheet for one shipment stated different quantities: exactly the disagreement the feature exists to prevent. One `formatQuantity` in `form-utils` now serves both forms, treating three decimals as the floor rather than the ceiling. **Named places were being thrown away whole.** `isNamedPlace` rejected the entire remainder when a freight qualifier appeared anywhere in it, so `CIF Rotterdam Prepaid` left box 15 empty with no warning. Replaced by `namedPlaceFrom`, which truncates at the qualifier and keeps `Rotterdam`. It also drops punctuation-only remainders, so `EXW.` no longer writes `.` into a form box. **CEVA dropped an operator's named place** whenever the term itself did not parse (`Ex Factory` + `SFO` wrote the term alone), and raised a conflict warning against freight wording that was never a competing place. The term written to special instructions is now built from the rule and the place that applies, and the comparison goes through `namedPlaceFrom`. **The keying sheet read the wrong SLI row.** `aggregateLines` splits rows on the canonical unit as well as D/F and the export-control triplet, so one commodity number can be two rows filing two units. Keyed on the code alone, the kilogram group was told it should be filing NO and the sheet asserted it had no figure for it — about goods the SLI files in KG. The lookup is now keyed on code and unit, with the code alone as a fallback. Also: box 24 appended the unit to plain piece counts under the 51 commodity numbers the Census file reports in `PCS`; the grid blanked its quantity total whenever two groups spelled one unit differently (`PCS` beside `EA`); the unit-of-quantity failure message hard-coded `KG` and so named the wrong remedy for tonne and gram codes; `basisNote` compared raw spellings and reported a row as deviating from a unit it was actually filing; `parseIncoterm` ran twice per fill; and `keyedQuantity` re-implemented the identity test `restateQuantity` already performs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013x3f6KRnFFe1R83BSTZewU
1 task
JoelA510
added a commit
that referenced
this pull request
Aug 20, 2026
Follow-up review rounds on #50 — quantity precision, Incoterm recording Four review rounds after #50. The headline defect: #50's precision fix never reached the printed form, because both adapters re-rounded the restated quantity to three decimals — a tonne-reported code printed 0.004 against a 4.263 kg row while the keying sheet kept the full figure. Also withdraws the named-place extraction heuristic in favour of leaving the box for the operator with a warning, and keeps `roundTo` untouched so the quantity work cannot move customs values. The fixture-backed suite has not run against any of this; it is gated on shipment documents that are not committed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request: Unit of quantity, and Incoterms that reach the paper
📋 Summary
Two ways a generated SLI could state something the shipment does not. Several Schedule B numbers are reported in kilograms while every invoice in this trade counts pieces — the app has always warned about that and then filed the piece count anyway. And the CEVA form's Incoterm was ticked by matching the raw string against the box names, so a term carrying its named place (
DAP Singapore) ticked nothing at all, producing a form with no delivery term on it and no sign of the omission.Each commodity row is now filed in the unit its commodity number requires, changeable before generating; and the Incoterm is read as a rule rather than matched literally, with its named place preserved.
✨ Highlights
reconciledecides the unit each row is filed in and restates the quantity into it. Both SLI adapters and both keying sheets read that decision, so the form and the sheet prepared for one shipment cannot state different quantities for the same goods.9031.90.0000on the sample shipments is exactly this.NO+KG), where only the filer knows how the goods are actually measured. Units the shipment cannot state are listed disabled with the reason rather than hidden — "there is no net weight for these goods" is the answer somebody is looking for.DAP Singapore,FOB Origin - Collect,cif rotterdamandEx Worksall resolve to the rule the form has a box for. One reader insrc/domain/incoterms.ts, shared by both adapters and by the Vendor A trade-terms parser.Origin - Collect) is kept out of a box captioned "NAMED PLACE/PORT".DATandDDUare named with their Incoterms 2020 replacement and left unticked. Reclassifying a delivery term is the filer's decision, not the adapter's.🗺️ Roadmap Progress
sb-uomcheck now judges the unit a row is filed in rather than the one the invoice printed.🏗️ Architecture Decisions
Key Patterns & Decisions
reconcile, not in the adapters. Only the Census dataset knows what a code requires, andreconcileis the one place that holds it. Adapters readreportingUom/reportingQuantityoff the row. Two implementations of "what unit is this?" is how a form and a keying sheet come to disagree.9031.90.0000in kilograms" holds for every row carrying it and survives a regrouping that splits or merges rows.quantityandsourceUomstill say what was printed, so every totals check still proves the rows against the source.reportingQuantityis a separate field, not a mutation.canonicalUnitexists to make what a document prints comparable with what the Census file requires. Applying it to the file's side too would fileNOagainst the 51 commodity numbers the file reports inPCS.0.004tonnes is 4 kg, not 4.263.nullrather than guessing.restateQuantityrefuses where nothing on the row supports the figure — no net weight, a weight that cannot be counted back into pieces, a content-mass unit.parseIncotermis domain knowledge, not carrier knowledge. It lives insrc/domain/incoterms.tsso that both adapters and the Vendor A parser share one answer; the parser previously carried a second, unvalidated copy.4.263on its own, in a column that has held piece counts on every form before it, is a number nobody can place.Logic Flow / State Changes
graph TD A["Census file: units per code"] --> B["reconcile"] C["CIPL: quantity, uom, net weight"] --> B D["Review screen: chosen unit per code"] --> B B --> E["SLILine.reportingUom + reportingQuantity"] E --> F["CEVA box 24"] E --> G["Nippon boxes 24 + 25"] E --> H["FedEx / UPS keying sheets"] E --> I["sb-uom check"]🔍 Review Guide
🚨 High Risk / Compliance Sensitive
src/domain/units.ts— the conversion rules themselves. What is convertible, what deliberately is not, how the precision scales, and how a derived figure is labelled.src/domain/reconcile/index.ts— where the unit is chosen per row, and thesb-uomcheck now judging the filed unit rather than the printed one.src/carriers/ceva/adapter.ts— box 24, and the Incoterm block that decides what gets ticked, what gets written out, and what gets warned about.src/domain/incoterms.ts— new. The rule list, the written-out phrasings, and the named-place / freight-qualifier distinction.🧠 Medium Complexity
src/carriers/keying-sheet/index.ts— restating each keying group from its own figures rather than apportioning the SLI row's; the blank quantity total on a mixed-unit sheet; the Note and Notes entries that say which rows are not piece counts.src/features/review.tsx— the unit picker and the note beside each filed figure.src/domain/schedule-b/index.ts— the expanded alias table and the reworded unit check.src/domain/cipl/parse-vendor-a.ts—incotermFromTradeTermsrouted through the shared reader.🟢 Low Risk
src/domain/types.ts,src/domain/reconcile/lines.ts— new fields, source-based defaults, androundTore-exported from its single definition.src/App.tsx,src/features/manual-fields.tsx— state wiring; the named-place field is no longer Nippon-only, since CEVA can now record one.README.md,roadmap.md.🧪 Verification Plan
1. Environment Setup
npm install— no new dependencies.2. Manual Verification
9031.90.0000(reported in KG).PCS (as invoiced)); the filed figure returns to the piece count, the note says it is by your choice, and the Schedule B unit check turns to a warning.4.263 KGfor that row and a bare count for the others.DAP Singapore, generate the CEVA SLI — theDAPbox is ticked and special instructions readIncoterm: DAP Singapore.DAPoption is chosen and box 15 readsSingapore.FOB Origin - Collect,FOBis ticked and box 15 stays empty — "Origin - Collect" is a freight term, not a port.DATterm: no box is ticked, the term is written out, and the warning names DPU as its replacement.3. Automated Tests
Verified in a real browser against a synthetic CIPL: upload, the picker's options and disabled states, and the filed figure following the selection.
carriers.test.tsasserted box 24 of the filed vendorA3 SLI as10\r75\r12. Its third row is9031.90.0000, which the Census file reports in kilograms — so the12that was filed was the reporting error this PR fixes, and the expectation is now10\r75\r4.263 KGwith that reasoning recorded beside it. This suite is gated on shipment documents that are not committed, so it could not be run here; it is worth a run where the fixtures are present.🔁 Review round (commit
a4e48ff)A review pass over the first commit found ten issues, all fixed in
a4e48ffwith regression tests. The ones worth knowing about:0.004— 6% of the declared weight lost to rounding. Places now scale with the conversion factor.source, so the review screen said "As invoiced" about a figure the document does not carry. Newconvertedbasis, surfaced everywhere a filed quantity is explained.PCScodes filedNO.scheduleBUnitwas being canonicalised, filing a unit the Census file does not list for those 51 codes.Xcodes keyed as zero. The keying sheet had no case for the eight commodity numbers filed with no quantity, keying a literal0against goods with a value.FOB Origin - Collectput "Origin - Collect" into box 15; and CEVA preferred the document's place while Nippon preferred the operator's, so the same two inputs filed two ways. Both now take the operator's, and CEVA warns when it overrides the document.📉 Detailed Changelog (Collapsible)
src/domain/units.ts:restateQuantity,canRestate,resolveReportingQuantity,roundTo, scale-aware rounding, the kilogram and item unit families, and theQuantityBasistype.src/domain/incoterms.ts(new):parseIncoterm,RETIRED_INCOTERMS,isNamedPlace.src/domain/types.ts:SLILinegainsscheduleBUnits,reportingUom,reportingQuantity,reportingBasis.src/domain/reconcile/index.ts:reportingUnitsoption; per-row unit resolution after aggregation; classification check passes the filed unit and basis.src/domain/reconcile/lines.ts: source-based defaults for the new fields;roundTore-exported fromunits.src/domain/schedule-b/index.ts: unit aliases for grams, tonnes, dozens, gross, hundreds and thousands;sb-uomjudged on the filed unit, with distinct messages for "no weight available" and "changed by hand".src/domain/cipl/parse-vendor-a.ts:incotermFromTradeTermsvalidated throughparseIncoterm.src/carriers/ceva/adapter.ts: Incoterm resolved and recorded; named-place precedence and conflict warning; box 24 carries the reported unit.src/carriers/nippon-express/adapter.ts: boxes 24/25 from the reported unit; named place defaulted from the term when it is one;normaliseIncotermbecomesformOption.src/carriers/keying-sheet/index.ts: rows keyed in the SLI's unit, restated per group; unit price per keyed unit; blank quantity total on a mixed-unit sheet; Note-column and Notes-tab entries for weight-keyed, converted and no-quantity rows.src/features/review.tsx: Filed qty & unit column,ReportingUnitPicker,basisNote.src/features/manual-fields.tsx: named place shown for CEVA too, with its own hint.src/App.tsx:reportingUnitsstate, reset on parse, fed toreconcile.src/domain/units.test.ts,src/domain/reconcile/reporting-units.test.ts: new suites, no fixtures needed.src/carriers/carriers.test.ts: Incoterm reading, named-place handling, CEVA recording, Nippon boxes 24/25, and an end-to-end document → form → sheet suite built on the synthetic CIPL.🤖 Generated with Claude Code
https://claude.ai/code/session_013x3f6KRnFFe1R83BSTZewU