Repository navigation
Conversation
725b4ea to
b95b073
Compare
0fcc771 to
1cd3b74
Compare
|
@coderabbitai review |
|
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
🔎 Reviewing |
b95b073 to
b9adaaa
Compare
1cd3b74 to
6eb92c2
Compare
|
@coderabbitai review |
|
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
🔎 Reviewing |
There was a problem hiding this comment.
This PR changes how the bot treats a persisted inventory snapshot event that fails to apply at boot. Before, hydrate_inventory dropped such an event without a log, and boot still started the liq_ inventory series, so a missing balance could show as zero on the dashboards. Now each failed event is logged by event type (no balances), counted in a HydrationReport, and makes the hydration Incomplete. restore_inventory_at_boot itself starts the series, and only after a complete hydration and after the pending-order gate recovery. The trading view is restored the same way as before, so only the metrics change.
My read: correct and well scoped. Things I checked:
- Routine data does not trip the new gate. Stale, inflight and watermark skips return
Ok.DeferredSnapshotReconciliationis only a force-apply witness and is never returned as an error. Legacy redemptions and stranded redemptions are filtered out before they are applied. - So the dashboards should not go dark on a normal boot. Only bad persisted values, such as a negative inflight, keep them off.
- The write lock is not held across an await.
- The new version reads a database the old one wrote, because nothing persisted changed.
- The boot tests drive the real seam with a publisher and cover all three outcomes. The snapshot test proves that hydration continues past a failed event.
Earlier threads (log level, balance payload in logs, return type, restart wording, Hydration visibility) are all handled or settled at this head. No new findings.
claude-opus-5-5 · high · 32 min
6eb92c2 to
0b4fe3a
Compare
b9adaaa to
33aef0c
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
There was a problem hiding this comment.
At boot, the bot rebuilds its inventory view from saved snapshots. Before this PR, an event that failed to apply was dropped without a log, and the liq_ inventory series still started. A missing balance could then show as zero on the dashboards. Now the failure is logged and counted, the rest of the snapshot still loads, and the series stay off for that run, the same as when a snapshot cannot be read. restore_inventory_at_boot now starts the series itself, after hydration and the open-hedge gate recovery, so nothing can start them before the view is restored.
My overall read: the change does what it says, and I found no new defects. Trading sees the same view as before, because the bad event is still skipped and later events still apply. The write-guard publish and the 60 second republish both stay idle until the start runs, so a partial view cannot leak out through those paths. The inventory write lock is released before the start takes its read lock, so boot cannot deadlock. There is no schema or persisted-state change, so upgrading is safe. The tests run the real boot seam and cover a good snapshot, an unreadable snapshot, and an event that fails to apply, including an event after the failure that still applies. The earlier threads (payload logging, the return type, the restart wording) are fixed at this commit.
One candidate was checked and dropped: the new warning logs the apply error, which can carry one share quantity. That quantity has no symbol attached, and other warnings in the code already log the same errors, so it is not a balance leak.
Note: this review covers 1cd3b74. The current head 0b4fe3a is the same change restacked on a newer base, and the PR's three files are identical in both.
claude-opus-5-5 · high · 3 h 13 min
0b4fe3a to
a4239d6
Compare
33aef0c to
34fd957
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
a4239d6 to
f40853a
Compare
34fd957 to
a1cc7ff
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
Part of RAI-3133.
At boot, a persisted inventory snapshot event that fails to apply now keeps the
liq_inventory series off, the same as a snapshot that cannot be read. Before, the bot dropped that event without a log and still started the series, so the missing balance could read as zero.Live effect: next release. Such a boot logs a warning per event. All
liq_inventory series then stay off until a clean restart; later polls do not turn them on. Trading and the inventory view do not change · Risk: low (boot, metrics start only) · Ships: next bot release · Blocks: needs #1707 below itDecisions
restore_inventory_at_bootnow starts them itself, after it restores the view. So the start cannot run first. conductor.rsProof