feat(compliance): seed dated delivery simulations - #6569
Conversation
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated path touched, human/CODEOWNERS review required.
This PR adds an optional delivery_date field to simulate_delivery and half-open [start_date, end_date) aggregation for dated batches in the compliance training agent. Validation runs before any mutation, numeric metrics sum with last-wins extension fields, and it correctly carries a minor changeset. No critical or high findings surfaced.
Why escalate (row 2): The PR modifies static/schemas/source/compliance/comply-test-controller-request.json, which matches the static/schemas/source/** gated-path glob (gated_paths: true). review_decision is REVIEW_REQUIRED, not APPROVED, so the hard approval gate is not satisfied. A real human/CODEOWNERS approval is required before this can merge.
Medium findings:
- static/schemas/source/compliance/comply-test-controller-request.json:778 —
delivery_dateadded to schema but missing from the comply-test-controller params doc (schema↔docs coherence gap; should be reconciled).
(Low: task-handlers.ts:8706 — get_media_buy_delivery end_date flips from inclusive end-of-day to exclusive half-open; noted for the human reviewer's attention.)
Escalation reason: modifies gated file static/schemas/source/compliance/comply-test-controller-request.json; requires human/CODEOWNERS approval (review_decision is REVIEW_REQUIRED).
Medium findings
- static/schemas/source/compliance/comply-test-controller-request.json:778 — delivery_date added to schema but missing from comply-test-controller params doc
Why human review
- Modifies gated path static/schemas/source/compliance/comply-test-controller-request.json (matches static/schemas/source/**) — human/CODEOWNERS approval required; review_decision is REVIEW_REQUIRED, not APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/compliance/comply-test-controller-request.json (modified) matches
static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
358071e to
1c3114c
Compare
|
Addressed Ladon’s medium schema↔docs finding in 1c3114c: the controller docs now include |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated path requires human/CODEOWNERS approval.
This PR modifies static/schemas/source/compliance/comply-test-controller-request.json, which is under a hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so Ladon cannot auto-approve regardless of how clean the diff is (decision table row 2).
Progress since prior run is acknowledged: the prior medium finding (delivery_date documented in schema but not in comply-test-controller.mdx) is resolved — the mdx now documents the field, request example, half-open interval, and response shape. The delta adds an optional delivery_date to simulate_delivery with a correct minor changeset, calendar-date validation, a SESSION_ENTRY_CAP guard, and matching tests. This is a net improvement.
Medium findings
- server/src/training-agent/task-handlers.ts:8706 — get_media_buy_delivery end_date silently flips inclusive→exclusive for all date-bounded reads, undocumented in the task schema. Worth a human look during the required review.
Escalation reasons
- Modifies gated path
static/schemas/source/compliance/comply-test-controller-request.json(matchesstatic/schemas/source/**);review_decisionisREVIEW_REQUIRED, notAPPROVED. Human/CODEOWNERS approval required before merge.
Medium findings
- server/src/training-agent/task-handlers.ts:8706 — get_media_buy_delivery end_date flips inclusive->exclusive for all callers, undocumented in the task schema
Why human review
- Modifies gated path static/schemas/source/compliance/comply-test-controller-request.json (matches static/schemas/source/**); review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/compliance/comply-test-controller-request.json (modified) matches
static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated path requires human/CODEOWNERS approval.
This PR modifies files under static/schemas/source/**, which is a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so per decision-table row 2 the gate is not satisfied and the outcome must be escalate.
The reviewer found no blocking findings in this run. Notably, the prior escalation's medium finding (task-handlers.ts:8706 — undocumented half-open date semantics) has been resolved: the schema now documents start_date inclusive / end_date exclusive with the equal-dates VALIDATION_ERROR constraint, matching the published get_media_buy_delivery.mdx task doc. The changeset is a correct minor (description clarifications only, no field/type/required/enum changes). This is a net improvement over the prior pass.
However, the schema-source approval gate is mechanical and cannot be lifted by a clean diff — only a recorded human/CODEOWNERS approval clears it.
Escalation reasons:
- Modifies gated path
static/schemas/source/compliance/comply-test-controller-request.json(matchesstatic/schemas/source/**); review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required. - Also modifies gated paths
static/schemas/source/media-buy/get-media-buy-delivery-request.jsonand.../get-media-buy-delivery-response.jsonunder the same gate.
Why human review
- Modifies gated path static/schemas/source/compliance/comply-test-controller-request.json (matches static/schemas/source/**); review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required.
- Also modifies gated paths static/schemas/source/media-buy/get-media-buy-delivery-request.json and get-media-buy-delivery-response.json under the same schema-source approval gate.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/compliance/comply-test-controller-request.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
|
Ladon cannot review this PR until merge conflicts are resolved. |
aa5b5dc to
40f99f3
Compare
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source paths require human/CODEOWNERS approval.
This PR modifies three files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so the gate is not satisfied and the PR cannot be auto-approved regardless of how clean the diff is (decision table row 2).
Escalation reasons
- Modifies gated path
static/schemas/source/compliance/comply-test-controller-request.json— schema-source approval gate; review_decision is REVIEW_REQUIRED, not APPROVED. - Modifies gated paths
static/schemas/source/media-buy/get-media-buy-delivery-request.jsonandget-media-buy-delivery-response.jsonunder the same gate.
Medium findings
- server/src/training-agent/task-handlers.ts:8790 — Empty-range rejection returns
INVALID_REQUEST; task reference maps the ordering violation toVALIDATION_ERROR.
Notes for the human reviewer: the reviewer assessed the schema/description changes as drift-removal aligning request/response descriptions with the already-published half-open, end-exclusive semantics in get_media_buy_delivery.mdx — not a new breaking wire change. Changesets are present. No critical/high findings. The single medium (error-code mismatch) does not block on its own but is worth resolving. This is a subsequent review of a prior escalate; the gate persists solely because review_decision is still not APPROVED — a recorded human/CODEOWNERS approval lifts it and the PR falls through to the normal table.
Medium findings
- server/src/training-agent/task-handlers.ts:8790 — Empty-range rejection returns INVALID_REQUEST; docs specify VALIDATION_ERROR
Why human review
- Modifies gated path static/schemas/source/compliance/comply-test-controller-request.json (matches static/schemas/source/**); review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required.
- Also modifies gated paths static/schemas/source/media-buy/get-media-buy-delivery-request.json and get-media-buy-delivery-response.json under the same schema-source approval gate.
- Hard, non-overridable path-based approval gate (gated_paths: true) is unsatisfied while review_decision is REVIEW_REQUIRED; cannot auto-approve regardless of diff cleanliness.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/compliance/comply-test-controller-request.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding
The developer resolved the prior medium finding: task-handlers.ts:8789 now returns VALIDATION_ERROR for an empty half-open date range, matching the docs and the get-media-buy-delivery-response schema. Credit for that fix.
However, the same error-code change introduces a new high-severity regression that must be fixed before merge.
Blocking findings
- server/src/training-agent/task-handlers.ts:8789 — VALIDATION_ERROR flip breaks the existing INVALID_REQUEST assertion this PR added at comply-test-controller.test.ts:1855 for the identical empty-range path; that test will now fail. Update the assertion to expect VALIDATION_ERROR (consistent with the docs/schema) so the suite passes.
Note: this PR also modifies gated schema-source paths (static/schemas/source/**) and review_decision is REVIEW_REQUIRED, so human/CODEOWNERS approval is still required before merge regardless of the above.
Blocking findings
- server/src/training-agent/task-handlers.ts:8789 — VALIDATION_ERROR flip breaks existing INVALID_REQUEST assertion added in comply-test-controller.test.ts:1855; the test will now fail
Fixed in cf4abbf: updated the stale controller assertion to VALIDATION_ERROR; 78/78 controller tests pass.
|
@ohalushchak-exadel #6569 is ready for the schema-gate review: the prior error-code/test finding is fixed and its change-request review was dismissed, all review threads are resolved, every check is green, and GitHub reports the PR mergeable. The only remaining blocker is the required human/CODEOWNER approval. |
Summary
delivery_datetosimulate_delivery[start_date, end_date)boundariesTesting
/saleshit a local SDK-runner discrepancy, while the identical clean RC21 GitHub job passes the required canonical storyboard 15/15Part of #2902