feat(reconciliation): add non-live static envelope - #369
Conversation
Co-Authored-By: Codex <noreply@openai.com>
|
Closing unmerged: required CI fails before this slice tests because the current main QPK dependency pin is behind QuantPlatformKit main\047s QPK_PIN contract. Aligning pyproject.toml/uv.lock is outside this frozen two-file scope; no remediation, rerun, merge, or deployment is authorized. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed2dce6835
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if any(envelope.get(key) != value for key, value in required_values.items()): | ||
| raise ValueError("non-live reconciliation envelope contains a value that is not allowed") |
There was a problem hiding this comment.
When an envelope is reconstructed externally or mutated before serialization, Python equality lets integer flags pass this validation because 1 == True and 0 == False. For example, setting learning_only and no_order to 1 and the eligibility/readiness flags to 0 produces canonical JSON and a digest containing numbers rather than the fixed boolean schema, which can fail downstream validation or alter consumers that distinguish JSON booleans from numbers. Validate both the value and its exact type for these flags.
Useful? React with 👍 / 👎.
| if key in envelope | ||
| } | ||
| ) | ||
| return json.dumps(envelope, ensure_ascii=False, separators=(",", ":"), sort_keys=True) |
There was a problem hiding this comment.
When a caller supplies any valid non-dict Mapping, such as MappingProxyType or UserDict, all preceding validation succeeds but json.dumps raises TypeError because it does not serialize arbitrary mapping implementations. This contradicts the public parameter type and also prevents reconciliation_envelope_digest from accepting the same documented input; convert the outer mapping to a plain dictionary before serialization.
Useful? React with 👍 / 👎.
1 participant
Scope
MISSING; retain negative learning-only/no-order/size-zero flags.Validation
uv run --frozen --extra test pytest -q tests/test_reconciliation_service.py tests/test_rebalance_service.pyuv run --frozen --extra test pytest -quv run --frozen --extra test ruff check .uv run --frozen --extra test python -m compileall -q application testsNo runtime, provider, credential, broker, account, order, configuration, workflow, or dependency changes. This PR must remain unmerged pending explicit authority.