SD 3384 SI issue fixes by palatsangeetha · Pull Request #569 · dcsaorg/Conformance-Gateway

palatsangeetha · 2026-09-23T20:39:28Z

No description provided.

…traight B/L, and Negotiable B/L scenarios

qodo-code-review · 2026-09-23T20:40:44Z

PR Summary by Qodo

Fix scenario-specific shipping instruction validation

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Adds scenario-specific SI scope checks and messages for Sea Waybill and B/L variants.
• Skips amended-status validation when required SI scenarios accept any updated status.
• Removes incorrect reference requirements and unsupported commodity fields from 3.0 request
 fixtures.

Diagram

graph TD
  A["Scenario Builder"] --> B["Submit Action"] --> C["Notification Validator"] --> D["Shared SI Checks"] --> E["Scope Validation"] --> F["Conformance Result"]
  A --> G["Get SI Action"] --> D

High-Level Assessment

The current approach is appropriate: it reuses the established SI_ANY sentinel to distinguish unrestricted amended status from null, which means the field must be absent, and centralizes conditional validation in EblChecks. Nullable expectations or duplicated per-action checks were considered but would weaken status semantics or fragment shared validation behavior.

Files changed (9) +147 / -46

Bug fix (7) +44 / -46

EblScenarioListBuilder.javaAllow any amended status in required SI retrieval scenarios +2/-2

Allow any amended status in required SI retrieval scenarios
• Required carrier and shipper SI scenarios now pass SI_ANY when retrieving shipping instructions. This prevents those scenarios from incorrectly requiring updatedShippingInstructionsStatus to be absent.
ebl/src/main/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilder.java

UC1_Shipper_SubmitShippingInstructionsAction.javaRelax amended-status expectations for SI submission notifications +2/-0

Relax amended-status expectations for SI submission notifications
• The UC1 submission notification check now uses SI_ANY for the updated shipping instructions status, avoiding an incorrect mandatory absence check.
ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/UC1_Shipper_SubmitShippingInstructionsAction.java

CarrierSiNotificationPayloadRequestConformanceCheck.javaSkip amended payload validation for unrestricted status +2/-1

Skip amended payload validation for unrestricted status
• Updated shipping instructions payload checks are no longer created when the expected amended status is SI_ANY. This keeps unrestricted scenarios from validating a condition they do not prescribe.
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java

EblChecks.javaCorrect SI scope, reference, and amended-status validation +38/-16

Correct SI scope, reference, and amended-status validation

• SI checks now validate transportDocumentTypeCode and isToOrder together with messages specific to Sea Waybill, Straight B/L, and Negotiable B/L scenarios. The shared checks also remove the incorrect SI-reference equality requirement and only validate updatedShippingInstructionsStatus when a concrete expectation exists.

ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java

ebl-api-3.0.0-negotiable-bl.jsonRemove unsupported commodity codes from negotiable B/L fixture +0/-9

Remove unsupported commodity codes from negotiable B/L fixture
• Removes nationalCommodityCodes from the negotiable B/L request example while retaining the HS code.
ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-negotiable-bl.json

ebl-api-3.0.0-request.jsonRemove unsupported commodity codes from default request fixture +0/-9

Remove unsupported commodity codes from default request fixture
• Removes nationalCommodityCodes from the default eBL 3.0 request example while retaining the HS code.
ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-request.json

ebl-api-3.0.0-straight-bl.jsonRemove unsupported commodity codes from straight B/L fixture +0/-9

Remove unsupported commodity codes from straight B/L fixture
• Removes nationalCommodityCodes from the straight B/L request example while retaining the HS code.
ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-straight-bl.json

Tests (2) +103 / -0

EblScenarioListBuilderTest.javaVerify required SI scenarios omit amended-status checks +53/-0

Verify required SI scenarios omit amended-status checks
• Adds carrier and shipper regression coverage confirming required Sea Waybill scenarios use SI_ANY and generate no updatedShippingInstructionsStatus checks.
ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java

EblChecksTest.javaTest scenario-specific SI scope validation +50/-0

Test scenario-specific SI scope validation
• Adds tests for the validation messages and accepted transportDocumentTypeCode/isToOrder combinations across Sea Waybill, Straight B/L, and Negotiable B/L scenarios.
ebl/src/test/java/org/dcsa/conformance/standards/ebl/checks/EblChecksTest.java

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved SI reference, wildcard-status, and any-SI scope validation issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Missing DSP reference equality check in carrier response validation · New
SI_ANY incorrectly skips allowed status value validation · New

What changed in this PR

This pull request updates eBL Shipping Instructions validation, scenarios, tests, and sample payloads.

Changes:

Adds SI scope validation and regression tests.
Updates wildcard amended-status handling.
Removes deprecated commodity-code examples from fixtures.

File	Summary
`ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java`	Tests required scenario status behavior.
`ebl/src/test/java/org/dcsa/conformance/standards/ebl/checks/EblChecksTest.java`	Tests SI scope rules.
`ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-straight-bl.json`	Removes deprecated sample data.
`ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-request.json`	Removes deprecated sample data.
`ebl/src/main/resources/standards/ebl/messages/ebl-api-3.0.0-negotiable-bl.json`	Removes deprecated sample data.
`ebl/src/main/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilder.java`	Uses wildcard amended-status expectations.
`ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java`	Adds scope checks and updates status/reference validation.
`ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java`	Updates nested notification validation.
`ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/UC1_Shipper_SubmitShippingInstructionsAction.java`	Configures notification status handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

qodo-code-review · 2026-09-23T20:44:30Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Wrong shipping instructions pass checks 🐞 Bug ≡ Correctness

Description

getSiPayloadChecks no longer compares a payload's shippingInstructionsReference with the
reference stored in the dynamic scenario parameters. A GET response or full-state notification can
consequently identify a different shipping instruction and still pass the remaining status, schema,
and content checks, with UC14 also lacking a separate root-reference match check.

Code

ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[L1939-1941]
-    checks.add(
-        JsonAttribute.mustEqual(
-            SI_REF_SIR_PTR, () -> dspSupplier.get().shippingInstructionsReference()));

Evidence
GET response validation delegates to getSiPayloadChecks, whose remaining checks no longer inspect
the shipping-instructions reference. Full-state notification payloads use the same method, and UC14
invokes notification validation without the separate reference-match check used by the other
notification actions; the schema permits the payload reference to be omitted but constrains a
present value only by shape, not by scenario identity.
ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/Shipper_GetShippingInstructionsAction.java[108-131]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1941-1964]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java[48-75]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/UC14_Carrier_ConfirmShippingInstructionsCompleteAction.java[38-48]
ebl/src/main/resources/standards/ebl/schemas/EBL_v3.0.0.yaml[4845-4860]
ebl/src/main/resources/standards/ebl/schemas/EBL_v3.0.0.yaml[5185-5197]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Removing the payload reference comparison allows a present but incorrect `shippingInstructionsReference` to pass, although restoring the old unconditional equality check would also incorrectly require the optional field.

## Fix Focus Areas
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1812-1816]
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1947-1964]
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/UC14_Carrier_ConfirmShippingInstructionsCompleteAction.java[38-48]

## Recommended Fix
Add a conditional comparison that permits `shippingInstructionsReference` to be absent but requires it to equal the scenario reference whenever present. Apply it in `getSiPayloadChecks` and add coverage for absent, matching, and mismatching values in GET responses and full-state notifications.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Status regressions escape the new test 📘 Rule violation ☼ Reliability

Description

requiredSiOnlyScenariosDoNotValidateUpdatedShippingInstructionsStatus asserts SI_ANY through
reflection and scans generated check titles instead of evaluating payload conformance. A checker
that rejects retained statuses under another title or accepts malformed ones still lets this test
pass, so the corrected blank-cell behavior is not protected.

Code

ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java[R110-112]
+    assertFalse(
+        carrierCheckTitles.stream().anyMatch(title -> title.contains("updatedShippingInstructionsStatus")),
+        carrierCheckTitles.toString());

Evidence
The checklist requires behavioral regression assertions when practical and rejects coverage that
relies only on internal implementation details. The added test accesses a private field reflectively
and checks display-title text without validating representative response or notification payloads.
AGENTS.md: Add Focused Behavioral Regression Tests for New or Corrected Behavior
ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java[95-115]
ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java[306-328]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The regression test examines a private field and check-title strings rather than proving the externally meaningful acceptance and rejection behavior for optional updated statuses.

## Fix Focus Areas
- ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java[95-115]
- ebl/src/test/java/org/dcsa/conformance/standards/ebl/checks/EblChecksTest.java[1329-1356]

## Recommended Fix
Add table-driven behavioral tests that execute the generated payload checks against absent status, each permitted retained status, and an invalid status. Assert conformance results directly, then remove the reflection and title-substring assertions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Updated notices bypass scope checks 📘 Rule violation ≡ Correctness

Description

CarrierSiNotificationPayloadRequestConformanceCheck.createSubChecks suppresses the entire
/data/updatedShippingInstructions validator when the expected status is SI_ANY, rather than
suppressing only exact-status matching. UC1 now supplies this sentinel, so a present updated payload
in its notification bypasses static carrier and document-scope checks that the synced specification
says apply to notifications.

Code

ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java[R68-69]
+                updatedShippingInstructionsStatus != null
+                    && updatedShippingInstructionsStatus != ShippingInstructionsStatus.SI_ANY,

Evidence
The authoritative Markdown states that carrier validations cover notifications and that applicable
Sea Waybill, Straight B/L, and Negotiable B/L scope validations also apply there. The changed
condition prevents getSiPayloadChecks, which installs those checks, from running at all for the
newly introduced SI_ANY notification expectation.
AGENTS.md: Conformance Behavior Must Match Authoritative Synced Specifications
specifications/confluence-sync/ebl/3.0.3/si-conformance-scenarios.md[158-177]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/action/UC1_Shipper_SubmitShippingInstructionsAction.java[183-187]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java[65-75]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1952-1968]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Using `SI_ANY` now disables every content validator for a notification's optional updated Shipping Instructions object, including static carrier and scenario-scope checks.

## Fix Focus Areas
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/CarrierSiNotificationPayloadRequestConformanceCheck.java[65-75]

## Recommended Fix
Build the updated-content checks whenever the expected status is non-null, retaining the wrapper's existing behavior of treating an absent or empty object as conformant. Let `getSiPayloadChecks` use `SI_ANY` only to omit exact-status matching, not the remaining content validations.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View high (1)

4. Invalid update statuses pass checks 📘 Rule violation ≡ Correctness

Description

getSiPayloadChecks and getSiNotificationChecks add UPDATED_SI_STATUS_ALLOWED_VALUES_CHECK only
in the branch skipped when the expected value is SI_ANY, even though required SI scenarios now use
that sentinel. A GET response or notification can therefore supply a present unsupported value such
as BOGUS without rejection because the schemas constrain the property only as a length-limited
string rather than an enum of the four documented update statuses.

Code

ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[R1957-1958]
+      checks.add(getUpdatedShippingInstructionsStatusCheck(updatedShippingInstructionsStatus));
+      checks.add(UPDATED_SI_STATUS_ALLOWED_VALUES_CHECK);

Evidence
The synced specification makes carrier workbook validations applicable to GET responses and
notifications, and the required scenarios now pass SI_ANY; in both validation builders, that value
conditionally omits the only explicit allowed-values check. The API schemas document four possible
updated statuses but structurally define the property as a length-limited string rather than an
enum, so schema validation cannot reject unsupported values.
AGENTS.md: Conformance Behavior Must Match Authoritative Synced Specifications
specifications/confluence-sync/ebl/3.0.3/si-conformance-scenarios.md[158-177]
ebl/src/main/resources/standards/ebl/schemas/EBL_v3.0.0.yaml[4883-4895]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilder.java[131-140]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1953-1959]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[2044-2050]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilder.java[129-140]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1953-1963]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[2009-2021]
ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[2043-2053]
ebl/src/main/resources/standards/ebl/schemas/EBL_v3.0.0.yaml[3936-3948]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The allowed-values validator for `updatedShippingInstructionsStatus` is skipped whenever `SI_ANY` is used, allowing an invalid present status to pass required GET response and notification checks.

## Fix Focus Areas
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[1953-1963]
- ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java[2043-2053]
- ebl/src/test/java/org/dcsa/conformance/standards/ebl/EblScenarioListBuilderTest.java[95-116]

## Recommended Fix
Keep exact-value or absence validation conditional on the expected status not being `SI_ANY`, but add `UPDATED_SI_STATUS_ALLOWED_VALUES_CHECK` unconditionally so every present status is restricted to the documented values. Update tests to verify that absent and valid update statuses are accepted under `SI_ANY`, while an invalid present value is rejected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Tickets: 🎫 EBL Conformance (SI): Update Scenarios & Validations (Carrier)

Review mode: ⚖️ Balanced: This changes conformance scenario behavior, validation logic, API message fixtures, and tests across multiple paths, creating meaningful contract and regression risk best assessed in one comprehensive review.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

palatsangeetha added 3 commits September 23, 2026 08:57

feat: Add scenario-specific SI validation messages for Sea Waybill, S…

18c6a94

…traight B/L, and Negotiable B/L scenarios

fix: remove incorrect mandatory SI validation checks

43af64d

SD- 3384 - Validation changes and fixes

e9776fc

palatsangeetha requested a lite review from Copilot September 23, 2026 20:39

Copilot started reviewing on behalf of palatsangeetha September 23, 2026 20:40 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java

Comment thread ebl/src/main/java/org/dcsa/conformance/standards/ebl/checks/EblChecks.java

qodo-code-review Bot reviewed Sep 23, 2026

View reviewed changes

SD- 3384 - fixed any si scenarios

1bb98a5

palatsangeetha merged commit 4c1d551 into dev Sep 23, 2026
1 check passed

palatsangeetha deleted the SD-3274-SI-updates-clean branch September 23, 2026 21:36

Navigation Menu

Repository navigation

SD 3384 SI issue fixes - #569

SD 3384 SI issue fixes#569
palatsangeetha merged 4 commits into
devfrom
SD-3274-SI-updates-clean

palatsangeetha commented Sep 23, 2026

Uh oh!

qodo-code-review Bot commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Sep 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

palatsangeetha commented Sep 23, 2026

Uh oh!

qodo-code-review Bot commented Sep 23, 2026

PR Summary by Qodo

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-code-review Bot commented Sep 23, 2026 •

edited

Loading