Carry snapshot diagnostics into execution#116
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c39416713a
ℹ️ 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".
| plan["portfolio"]["cash_by_currency"] = cash_by_currency | ||
| _attach_snapshot_diagnostics( | ||
| plan, | ||
| decision=normalized_decision, |
There was a problem hiding this comment.
Preserve original decision diagnostics when attaching snapshot data
When a no_execute decision carries snapshot manifest fields in decision.diagnostics, _normalize_to_value_target_decision replaces it with a synthetic hold-current decision whose diagnostics are empty, so passing normalized_decision here drops the very fields this helper tries to copy into execution. In that scenario the signal snapshot/notification still won't show the manifest dates or fallback status unless the same values also happen to be duplicated in runtime_metadata; use the original decision (or merge its diagnostics before normalization) for this attachment step.
Useful? React with 👍 / 👎.
Summary
Tests