feat(quoter-bot): ship flat monitoring event vocabulary - #170
Open
haydenshively wants to merge 12 commits into
Open
feat(quoter-bot): ship flat monitoring event vocabulary#170haydenshively wants to merge 12 commits into
haydenshively wants to merge 12 commits into
Conversation
quoter-bot already had the Better Stack transport, but nearly every per-cycle report lacked an `event` field and shipped as the `bot.action` catch-all carrying nested `LadderRunResult[]` and `SetupCheckReport` blobs. No metric expression can aggregate that shape, so the only working alert was "the bot crashed." Implements TIB-2026-08-23-quoter-bot-monitoring-events: a flat, named, low-cardinality vocabulary projected from data the bot already reads. - Domain stays pure. `generateLadderWithDiagnostics` and `decidePositionBootstrapWithDiagnostics` return clamp, clearance, truncation, and size-cap counts alongside their results; the existing entry points become thin wrappers with unchanged behavior. - `calculateLadderCapacities` also returns the balance primitives it previously discarded. The four capacities are saturating minima and cannot be inverted into a position value, so PnL needs the inputs. - Fills come from diffing monotonic per-group `consumed`, not the active quote set: reconciliation reserves fresh group IDs on every recenter and resize, so a quote-set diff conflates takers with the bot's own churn. Adds an optional observation-only `readConsumption` port. - Projections run at the four CLI cycle seams where the workflow is known statically, and ship through the existing `writeEvent` seam so stdout and Better Stack stay identical. - `bot.configured` anchors absence alerts; `schemaVersion` binds into logger context only when shipping is fully configured, preserving the both-or-neither opt-in and zero-network default. Monitoring adds no RPC round trip: `maturityTimestamp` is projected from groups already read, and consumption reuses the ownership read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
quoter-bot now imports createLogger, railwayContext, and classifyShippingConfig directly to bind schemaVersion into the shipping logger context, so the dependency is real and knip can see it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adversarial review found two gaps against the TIB. Fill telemetry added a real provider round trip. `readConsumption` calls `readGroups`, which was a plain function hitting the Morpho API on every call, and `completeResult` awaited it — so a verbose cycle paid an extra request per market and delayed the quoting loop. `readGroups` is now single-flighted, so the consumption read collapses into the active-quote read it already runs concurrently with. Deduplication is concurrent-only, so every fresh call still reads current group state. The `bot.action` fallback was still reachable. Raw cycle arrays and terminal monitor reports shipped without an `event`, so Better Stack kept receiving unaggregatable nested blobs alongside the new vocabulary. Shipping now filters to named records only. Terminal output is byte-identical, and the dropped content survives as `cycle.completed`, `guardrail.halted`, and the lifecycle events. Also corrects the reference's no-additional-RPC claim, which overstated what the consumption path does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`guardrail.spread-rejected` keyed off the sanitized `BootstrapAdapterError` classification, which collapses 32 distinct adapter operations into one name. Any adapter failure — a provider response, a transaction policy rejection, a stale reference — tripped a cross-book spread alert, masking the real failure class. Adds `operatorAdapterOperation`, an allowlist projection of the adapter's specific operation, and threads it onto the bootstrap failure and halt outcomes alongside the existing `errorName`. The guardrail now keys on `adapterOperation === 'negative-spread'`, so it fires only on an actual cross-book rejection. Both paths a negative spread can surface through are covered: a reconcile throw and a preview throw. The operation field is typed `string` on the error, so it is allowlisted rather than passed through — an operator-visible dimension must never be able to carry provider text. Unrecognized values are withheld. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An observability design review flagged that the implementation violated its own boundary, shipped redundant records, and missed the two incidents operators most need explained. Shipping is now an explicit allowlist rather than "any record carrying an `event`". The previous predicate admitted the nested `quoter-bot.cycle` envelope and `readonly.make`, so unversioned blobs still reached the log source under a different name. A compile-time exhaustiveness check fails the build if an event is added to the union without being listed. Adds `bot.failed`, the only signal for two failures no cycle record can describe: a readiness check that fails during startup before any monitor loop begins, and a fail-together lifecycle where one supervised workflow ends and stops its peers. The combined case emits one record per failed workflow, so "which workflow half-broke?" is answerable. Filtering shipping to named records had removed this information entirely. Fixes "not quoting", which produced silence: `book.observed` was skipped whenever no quote was active, so `state: 'empty'` only fired when a quote already existed. Both sides are now reported on every observed cycle. Splits per-market cadence into `market.configured`. One process-wide shortest interval made slower markets look overdue to absence alerts. Cuts fields that restate other fields or a constant: `setup.ready`, `bootstrap.progress.shortfallAssets` and `.mode`, `guardrail.cross-book-cleared.clearanceBps`, `guardrail.spread-rejected.errorName`, `transaction.settled.status`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`market.configured` mapped only the ladder strategy, so a market configured for bootstrap alone appeared in no manifest record once `bot.configured` stopped carrying `marketIds` — leaving it outside every absence alert. It now covers the union of both strategies, with `ladderIntervalSeconds` omitted for a bootstrap-only market whose cadence is the process-wide `bootstrapIntervalSeconds`. Also drops `mode` from the documented cardinality allowlist: no event carries it since `bootstrap.progress` lost the field, and records that the manifest is emitted by `start` rather than the standalone operator commands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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: a52c97567b
ℹ️ 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".
Eleven review findings; ten were real. Fill telemetry was lost on the cycles that matter most. Consumption was sampled inside `completeResult`, which runs after `reconcile` has already forgotten a replaced group from durable ownership — and a fill is exactly what causes the resize that replaces it. It is now sampled alongside the active-quote read, before reconciliation, sharing that read's deduplicated groups request. `book.observed` described the book before reconciliation, so a successful first publication reported `state: 'empty'` and a successful invalidation reported the old book as `quoting`. It now projects the post-check read, falling back to the pre-check one when that read did not succeed. `bestRateBps` took the maximum on both sides, but the sides run in opposite directions from the center, so every higher-side gauge was inverted — the outermost rung was labelled best. The fill baseline could regress. An eventually consistent replay of an older `consumed` lowered the baseline, so the next fresh value re-emitted the already-counted portion. Baselines now only advance, and are evicted after 500 unseen cycles so the map stays bounded as reconciliation keeps reserving fresh group IDs. Bootstrap `durationMs` spanned both passes of `runOnce`, so an early market's duration included every later market's read. It now measures only that market's own planning, mutation, and post-check work. Also: `cashBalanceAssets` reported `min(balance, allowance)` rather than the wallet balance it documents; `cappedAssets` could go negative against its unsigned contract; `market.configured` could not distinguish a market missing a bootstrap cycle from one never configured for bootstrap; `adapterOperationField` sat in a file containing a class, against the utility-isolation convention; and `groupRateBps` was documented as exact under `shared-rung` despite tick alignment moving the published rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A contract audit checked twelve properties this change asserts about itself and disproved eight. This closes the ones that were real and corrects the prose for the ones that were only claimed. Monitoring could still add a provider round trip. The consumption read and the active-quote read raced a single-flight slot that is released the moment the first request settles, so deduplication was timing-dependent — `readActive` awaits `cleanupRemovedMarkets` first, giving the consumption request time to finish and free the slot. One adapter call now derives both the quote and its groups' consumption from a single groups read, so the guarantee holds by construction rather than by scheduling. Monitoring could halt quoting. Monitor loops await the cycle callback while holding the combined writer queue, so a throwing projection or a failing sink surfaced as a failed cycle. Deriving and writing the derived records is now isolated: telemetry can be lost, the bot cannot be stopped by it. The cycle value keeps its unguarded write so genuine output failures still surface. Prose corrected where the code was right and the claim was not: terminal failure reports reach stderr, not stdout; the shipping opt-in silences log records but not the independently configured heartbeat; the allowlist governs records flowing through the CLI writer, while the four lifecycle records reach the logger directly; and `adapterOperation` is an internal discriminator, not a shipped dimension. Two alert recipes were unsound. Crash now covers `bot.unexpected-error`, which an unclassified entrypoint failure emits without any `bot.failed`. Stale reference now records that a bootstrap-only market can skip the rate read entirely, so absence there means "not needed", not "stale". Adds a TIB addendum recording every delta from the original event table rather than editing the accepted decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
marked this pull request as ready for review
August 25, 2026 06:48
haydenshively
force-pushed
the
feat/quoter-bot-monitoring-events
branch
from
August 25, 2026 06:51
c1aa10e to
252954a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1aa10e829
ℹ️ 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".
…oring-events Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
Bot
force-pushed
the
feat/quoter-bot-monitoring-events
branch
from
August 25, 2026 14:54
9e4514d to
b6ce8fc
Compare
Collaborator
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6ce8fcbbe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements TIB-2026-08-23-quoter-bot-monitoring-events. Closes BOTS-86.
Problem
quoter-bot already had the Better Stack transport —
createBotObservability, the heartbeat, processobservers, the both-or-neither opt-in. What it lacked was a vocabulary.
bot-observability.utils.tsdefaults any record without an
eventfield tobot.action, and nearly every per-cycle report thebot emitted lacked one, so Better Stack received nested
LadderRunResult[]blobs that no metricexpression can aggregate. This is a what-to-emit change, not a plumbing change.
What ships
A typed
MonitoringEventunion (17 variants,schemaVersion: 1bound into the logger context) withflat scalar payloads and bounded grouping dimensions. Records are projected from cycle results at the
application layer —
src/domain/stays pure and gets no logger. Guardrails that were deliberatelymade non-throwing (rate clamps, cross-book clearance, rung truncation, exposure caps) are now
visible, aggregated per side per cycle rather than per rung.
Fills come from per-group
consumeddeltas, which are monotonic per group ID, rather than fromdiffing the active quote set —
reconcilechurns group IDs on every recenter, so a quote-set diffconflates taker consumption with the bot's own cancel/replace.
Only allowlisted names reach Better Stack.
isShippableRecordcarries a compile-time exhaustivenessguard against the union, so a new variant that is not allowlisted fails typecheck;
quoter-bot.cycleand
readonly.makestay local to stdout. The four lifecycle records (bot.started,bot.stopped,bot.unexpected-error,heartbeat.failed) ship from the observability layer and bypass thisboundary.
Zero additional RPC. Every field is a projection of data already read, including
maturityTimestamp(derived from the groups the ladder already fetches) and the accounting primitives that
calculateLadderCapacitiespreviously consumed and discarded.Notable decisions
bot.failed— a readiness failure throws before any monitor loop starts, and the fail-togetherlifecycle can stop without any cycle record explaining which workflow ended. Without this record
the shipped stream shows a start and a stop with nothing between them.
market.configuredper market — one process-wide minimum cadence would make every slowermarket look overdue. Covers the union of ladder and bootstrap markets so a bootstrap-only market
stays in scope for absence alerting.
readActiveStatereturns the quote and its consumption together, and issampled before
reconcileforgets replaced groups. An earlier version deduped two reads with asingle-flight helper; that was timing-dependent, so the port was changed instead.
Cli.writeCyclewrites the raw record first, then projectsinside a
try/catch. Telemetry can be lost; a cycle cannot be stopped by it.hardHaltpulls every market, so itstransaction.settledrecords omitmarketIdeven though the halted result names the market thattriggered it. Only the market-scoped
market-read-failedreconciliation keepsmarketId.Docs: event tables, the units/cardinality contract, alert recipes, and Known limits in
bots/quoter-bot/README.mdandbots/quoter-bot/docs/reference.md.Addendum Aon the TIB recordsevery delta between the accepted design and what shipped.
Review
Three passes, each catching a distinct class of defect: a code review (shipping allowlist too
permissive, dropped terminal reports), a design review for necessity/sufficiency/simplicity (cut
derivable fields,
book.observedsilent in the empty case), and a contract audit that verified eachproperty this PR asserts end-to-end (the timing-dependent dedup, monitoring on the cycle path, and
six documentation claims that did not match the code).
Automated-review follow-ups:
position.observed/book.observednow describe the same post-checksnapshot, an expected
MakerAccountErrorships one sanitizedbot.failedinstead of nothing,cleanup-failedreports carrycleanup.errorName, and ladder hard-halt cancellations keep theirconfirmed hashes. Projecting one-shot
runOnceresults throughwriteCyclewas declined — that pathprints its full report to stdout, so it would interleave derived records into operator output.
Verification
pnpm --filter @morpho-org/quoter-bot run typecheck·pnpm lint(0 warnings) ·pnpm test(77 files; 2 RPC-fork E2E suites need
RPC_URL_8453) · knip ·scripts/check-jsdoc.ts— all clean.New guards were verified by breaking one assertion and reverting.
Known CI gap: the
Testjob fails atfoundry-rs/foundry-toolchainsetup(
foundryup: cannot execute binary file) before any test runs. The same failure is onmain(
b5624de) and this branch does not touch the workflow.Link to Devin session: https://app.devin.ai/sessions/aff4124e02814ac89fb7e6520ecc82c1
Requested by: @haydenshively