BKG 2.0: SD-3132: Add Special Instructions - #644
Conversation
PR Summary by QodoBKG v2: add specialInstructions at booking and requestedEquipment level
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
There was a problem hiding this comment.
Pull request overview
Adds support for specialInstructions free-text notes to the Booking API v2.0.5 schema, allowing shippers to provide ordered instruction lines at both the Booking root level and per requestedEquipment.
Changes:
- Introduces
specialInstructions(array of 1–5 strings, max 512 chars each) on Booking-level schemas (CreateBooking,UpdateBooking, andBooking). - Introduces
specialInstructionson equipment-level schemas (RequestedEquipmentandRequestedEquipmentShipper).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review by Qodo
1. Booking specialInstructions not string
|
| specialInstructions: | ||
| type: array | ||
| minItems: 1 | ||
| maxItems: 5 |
There was a problem hiding this comment.
1. Booking specialinstructions not string 📎 Requirement gap ≡ Correctness
The specialInstructions field is defined as an array of strings at both the booking level and the requested-equipment level, but the compliance requirement specifies a single optional free-text string field. This schema mismatch can break or invalidate client requests that implement the specified CreateBooking/UpdateBooking contract.
Agent Prompt
## Issue description
`specialInstructions` is currently modeled as an array of strings in the CreateBooking/UpdateBooking request schemas at both the booking level and the requested-equipment level, but compliance requires a single optional free-text field of type `string`.
## Issue Context
The SD-3132 compliance checklist specifies that `Special Instructions` must be an optional string field at the booking level (PR Compliance ID 1) and also an optional string field within the requested equipment structure (PR Compliance ID 2). The current OpenAPI/YAML schema definitions use `type: array` with string items in both locations, which can cause clients implementing the required contract to fail validation or break due to the type mismatch.
## Fix Focus Areas
- bkg/v2/BKG_v2.0.5.yaml[3187-3206]
- bkg/v2/BKG_v2.0.5.yaml[3618-3637]
- bkg/v2/BKG_v2.0.5.yaml[4118-4137]
- bkg/v2/BKG_v2.0.5.yaml[5464-5482]
- bkg/v2/BKG_v2.0.5.yaml[5588-5606]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| items: | ||
| type: string | ||
| maxLength: 512 | ||
| description: | |
There was a problem hiding this comment.
2. specialinstructions has maxlength 📎 Requirement gap ≡ Correctness
The schema enforces maxLength: 512 on specialInstructions items, but the requirement explicitly says not to enforce a maximum character length while the limit is TBC. This can cause valid longer instructions to be rejected by schema validation.
Agent Prompt
## Issue description
`specialInstructions` enforces `maxLength: 512`, but compliance requires no explicit maximum length until specified.
## Issue Context
The SD-3132 compliance checklist states the maximum character length is TBC, so adding a concrete max length risks premature validation failures.
## Fix Focus Areas
- bkg/v2/BKG_v2.0.5.yaml[3201-3204]
- bkg/v2/BKG_v2.0.5.yaml[3632-3635]
- bkg/v2/BKG_v2.0.5.yaml[4132-4135]
- bkg/v2/BKG_v2.0.5.yaml[5476-5479]
- bkg/v2/BKG_v2.0.5.yaml[5600-5603]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| items: | ||
| type: string | ||
| maxLength: 512 |
There was a problem hiding this comment.
3. Empty instruction strings allowed 🐞 Bug ≡ Correctness
specialInstructions items only constrain maxLength, so an array can schema-validate while containing empty or whitespace-only strings (including newline-only entries), resulting in a “non-empty” instructions list with no actual instruction content.
Agent Prompt
## Issue description
The new `specialInstructions` array item schema only specifies `type: string` + `maxLength`, which permits empty/whitespace-only strings. Because the array also has `minItems: 1`, callers can satisfy validation while providing no meaningful instruction text.
## Issue Context
This behavior is introduced in multiple repeated schema blocks (booking-level and requestedEquipment-level). Most other “free text” fields in this spec use a non-whitespace `pattern` to ensure the value contains real content (e.g. `^\S(?:.*\S)?$`).
## Fix Focus Areas
- bkg/v2/BKG_v2.0.5.yaml[3187-3206]
- bkg/v2/BKG_v2.0.5.yaml[3618-3637]
- bkg/v2/BKG_v2.0.5.yaml[4118-4137]
- bkg/v2/BKG_v2.0.5.yaml[5464-5480]
- bkg/v2/BKG_v2.0.5.yaml[5588-5604]
## Suggested fix
Add a constraint ensuring at least one non-whitespace character is present while still allowing leading/trailing whitespace and embedded newlines. For example:
- `pattern: '^[\\s\\S]*\\S[\\s\\S]*$'`
Apply consistently to all `specialInstructions.items` definitions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| **Condition:** The order of the items in this array **MUST** be preserved | ||
| as by the provider of the API. |
There was a problem hiding this comment.
4. Ambiguous order responsibility 🐞 Bug ⚙ Maintainability
The specialInstructions description says order must be preserved “as by the provider of the API”, which is ambiguous in request contexts where the shipper/consumer supplies the list and can confuse implementers about who must preserve ordering.
Agent Prompt
## Issue description
`specialInstructions` includes an ordering requirement with awkward/ambiguous phrasing ("preserved as by the provider of the API"). In request schemas, the list is provided by the consumer/shipper, while the API provider may need to preserve ordering when storing/returning.
## Issue Context
The wording appears in multiple newly added blocks at booking and requestedEquipment levels.
## Fix Focus Areas
- bkg/v2/BKG_v2.0.5.yaml[3192-3201]
- bkg/v2/BKG_v2.0.5.yaml[3623-3631]
- bkg/v2/BKG_v2.0.5.yaml[4123-4131]
- bkg/v2/BKG_v2.0.5.yaml[5469-5476]
- bkg/v2/BKG_v2.0.5.yaml[5593-5600]
## Suggested fix
Replace with explicit responsibility, e.g.:
- "The API provider MUST preserve the order of the items in this array."
(or if intended differently, explicitly name consumer vs provider).
Apply consistently across all occurrences.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
SD-3132: Add
specialInstructionson root level and onrequestedEquipmentlevel