Repository navigation
Conversation
|
@coderabbitai full review |
![rain-marvin[bot]](https://avatars.githubusercontent.com/in/5077017?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR lets the liquidity board's log panels read the bot's own liq_trade, liq_transfer and liq_event lines behind a Source switch, with exporter still the default. It adds the per-row event timeline and the query-state notice in the detail dialog.
Since the last review (0d461c6): the head was restacked onto the parent PR again. The PR's own commit is unchanged in every hand-written file; git range-diff shows only the generated board JSON differs, because it embeds the parent's new recovery-commands.js text for the Alpaca to Base converting resume (it now asks the operator to confirm at Alpaca that no USD to USDC order is still open before the resume, and limits the hand settlement to a ConversionFailed end). That text and its transfer.ts twin belong to the lower PR and are reviewed there. On acb3654, gen-t0-liquidity.py --check passes (5 boards match the generator) and the transfer-board parity and transfer Vitest suites pass (90 tests). I found no new defects.
Earlier findings: my last review had no open findings, and nothing has regressed.
Threads resolved by hand:
- The narrow Stage 4 precondition (docs/observability.md:446, resolved by JuaniRios): addressed. The missing items are on the RAI-3000 checklist, which gates the flip.
- The preconditions not being written down (docs/observability.md:446, resolved by JuaniRios): addressed in the same RAI-3000 comment.
- CodeRabbit's point that an uncertain refresh releases the env-switch hold (t0-liquidity.json:2741, resolved by JuaniRios): dismissed for now, and the reason still holds. That code is unchanged on this head. It needs Grafana to drop a failed table query from its error list, the worst result is a wrong "not in the newest 500" note with no commands shown, and it goes with the masked table-query errors follow-up in RAI-3000. CodeRabbit agreed to defer it.
The other resolved threads were withdrawn by CodeRabbit or resolved by me with a reply that still holds. Nothing blocks this PR.
claude-opus-5-5 · high · 1 min
6421f57 to
e0ddd8c
Compare
acb3654 to
c5f3d9c
Compare
|
@coderabbitai review |
|
@rain-marvin approve |
✅ Action performedReview finished.
|
|
🔎 Verifying that my findings from |
There was a problem hiding this comment.
This PR lets the liquidity board's log panels read the bot's own liq_trade, liq_transfer and liq_event lines behind a Source switch, with exporter still the default. It adds the per-row event timeline and the query-state notice in the detail dialog.
Since the last review (acb3654): this head is only a restack onto the parent PR. git range-diff shows that the PR's own commit is the same in every hand-written file. Only the generated board JSON changed, because it embeds the parent's new recovery-commands.js text: after the nonce-closing step of the Base to Alpaca fail, an operator whose withdrawal mined is now told to start the bot, run resume-usdc and not fail the transfer. The guide's resume-usdc entry also asks for an Alpaca order check before a resume at converting. That text and its transfer.ts and recovery-guide.json twins belong to the lower PR and get their review there. On c5f3d9c, gen-t0-liquidity.py --check passes (all 5 boards match the generator), and the transfer and transfer-board Vitest suites pass (91 tests). I found no new defects.
Earlier findings: my last review had no open findings, and nothing has regressed.
Threads resolved by hand:
- The narrow Stage 4 precondition (docs/observability.md:446, resolved by JuaniRios): addressed. The missing items are on the RAI-3000 checklist, which gates the flip.
- The Stage 4 preconditions not being written down (docs/observability.md:446, resolved by JuaniRios): addressed in the same RAI-3000 comment.
- CodeRabbit's point that an uncertain refresh releases the env-switch hold (t0-liquidity.json, resolved by JuaniRios): dismissed for now, and the reason still holds. That code has not changed on this head. The case needs Grafana to leave a failed table query out of its error list. The worst result is a wrong "not in the newest 500" note with no commands shown, and the fix goes with the masked table-query errors follow-up in RAI-3000. CodeRabbit agreed to defer it.
All other resolved threads were withdrawn by CodeRabbit or resolved by me with a reply that still holds. Nothing blocks this PR.
claude-opus-5-5 · high · 13 min
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
@JuaniRios 🔔 Follow-up: a day after my review @JuaniRios, this PR has been quiet since the approvals on Next step: get an engineer to approve. #1696 (Graphite) needs one too, since this PR sits on top of it and is still waiting for its own human approval. claude-opus-5-5 · high · 1 h 54 min |
This branch has not been deployed
1 participant
Part of RAI-3064.
The board's log panels now follow
Sourcetoo. Onbotthey read the bot's ownliq_trade,liq_transferandliq_eventlines and its own log lines, instead of the exporter's logs. Item 6b of the RAI-2998 stack, on top of #1696.Live effect: none until
Sourceisbotand the bot logs JSON toliquidity-bot. The default (exporter) shows the same rows as today. After a failed refresh, the detail dialog can show a new out-of-date notice on either source · Risk: low to medium (read-only board, but every refresh on the default source also runs the hidden bot queries, see Decisions) · Ships: on merge, through theobservabilityworkflowWhy
6a left Trades, Rebalances, the detail dialog and the Logs tab on the exporter's logs, and #1654's USD columns and event timeline were waiting for real bot lines. #1696 writes those lines, so the board can read them.
Decisions
exporter-<name>andbot-<name>, and its first transformation isfilterByRefIdon${source:text}-<name>. Grafana fills variables into transformation options, and the refId match is exact.check_log_sourcesin the generator fails on any log panel that doesn't follow this. Allowed exceptions: the dialog's bot-onlybot-events, and Orders, which stays on the exporter's log because the bot writes no order lines.gcloud logging readon prod), plus one more list call per panel against the project's read quota. The plugin shows a throttled call as missing rows, not as an error. I found no clean way to skip a query by$sourcealone: Grafana's Mixed datasource skips a query whose datasource uid resolves to empty, but no format of$sourcegivescloudlogfor one source and empty for the other. A second variable would do it, at the price of two switches. It's in the docs. lmk if you'd rather have that, or a generator flag flipped at Stage 3.usd) to Trades and Rebalances. A bot line's body is its message, not its JSON, so the Rebalances Type column reads the bot's labels as JSON text (a step limited to the bot frame).occurred_at. Documented in the panel and the docs.log-lines.js, new, prepended like the other helpers):event_idit already has.resource.labels.project_id, which both sources carry). This closes the 6a gap where a cancelled query after an Environment switch released the hold with the other project's rows. The hold itself stays.event_id, not by time.liq_event, filtered by parent and transfer kind, not by venue, since a venue correction changes it. Payload is parsed from its JSON string.bot-events, doesn't. The timeline says when the event query failed. The event check reads the normalized error list, a singleerrortoo.bot-events. When only errors that do not block are listed, a red notice says the row and its CLI commands can be out of date. Refresh before you run a command. log-lines.jsProof
gen-t0-liquidity.py --checkpasses. The new check fails as expected when I drop apick_source, renamebot-events, or point a bot query at an exporter log.queryState,lineEntries(labels vs JSON body, project filter,event_iddedupe),eventTimeline,timelineIncomplete,blockingErrors, and the sequence ordering inlatest(). eslint and svelte-check clean, pre-commit clean.afterRenderin happy-dom for both sources: bot trade with timeline, other-project rows, exporter ignoring bot frames, bot transfer with status history, a failed hidden query, a timeline missing sequence 1, and a failedbot-eventswith an old frame.gcloud logging readon prod, read-only).byRefIdmatcher, frame-scopedfilter,convertFieldTypeto string (JSON.stringifyfor objects) and to number (""becomes null), the plugin's frame shape and labels.bot-eventsrefresh with an old frame was silent) and two lows, after the cap. I fixed those and an opus fix-verifier plus the tests checked them, but no fifth panel pass ran.log_format = "json"and T0Trade/t0.devops#792 applied first. Also not checked live: the extra latency and quota on the default source with several viewers open.Merge conditions
Same as the stack below. This deploys on merge to master through the
observabilityworkflow, not with a bot release. That's safe before the bot logs JSON, becauseSourcedefaults toexporter. Before Stage 4 flips the default, open the board withSource = boton staging and check every log panel has rows.