(MOT-4365) feat(harness,console): show the payload a trigger fire delivered - #718
Merged
Conversation
The trigger_fired payload is deliberately uncapped, so an oversized one (e.g. a bulk UPDATE...RETURNING event) is the likeliest way the append ever fails. Losing the whole record then drops the fire notice from the timeline and the `retired` flag the panel needs for a fired once-trigger's ghost row. `emit` now retries once with `payload: None` on append failure when the record carried a payload, reusing the same entry id so a partially-succeeded first attempt is harmless. Also pin, in the mechanical_reaction e2e scenario, that a delivered call binding's fired record actually carries a `payload` key - nothing previously asserted this, so flipping it away silently would have left the suite green.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 54 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughChangesTrigger-fired payload flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TriggerDelivery
participant FiredRecord
participant SessionMapper
participant ChatTerminal
TriggerDelivery->>FiredRecord: store delivered payload
FiredRecord-->>SessionMapper: provide trigger-fired payload
SessionMapper->>ChatTerminal: render mapped payload
ChatTerminal-->>ChatTerminal: format JSON and copy payload
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Problem
The console's trigger-fired card showed registration and fire bookkeeping but never what was delivered. For a ƒ-call binding the dispatched payload existed only at fire time inside
harness::trigger::deliverand was never persisted — so "what was sent to the target function?" was unanswerable from the timeline. Wakes were only marginally better: the event was embedded in the notification text, truncated at 600 chars.The delivered value can differ from the raw fired event three ways: a condition's
Allow { payload: Some(next) }rewrites the event, projection injects it into the target'spayloadtemplate atevent_into, andfilesystem_scope::injectstamps the payload just before dispatch.What changed
TriggerFiredgains an optionalpayloadfield carrying the final dispatched payload — post-conditions, post-projection, post-stamping.dispatch/call_targetnow return(Value, Result<(), String>)so the record gets what actually went out, including on a failed dispatch (the attempted payload lands next to thedispatch failed: …note). Wakes record the post-conditions event, recovering what the 600-char truncation loses. Skip/gc/expiry records carry no payload — nothing was delivered.The console types the field, renders it as a
payloadpane in the card's terminal tab, and picks it up in the RAW JSON tab for free.Decisions worth knowing
fired::emitnow retries the append once withpayload: Noneif the first attempt fails. Previously a size-driven append failure would have dropped the entire record — losing the timeline notice and the panel'sretiredflag for a fired once-trigger. Bookkeeping survives even when the payload cannot be persisted; still strictly best-effort, never blocks a fire.Depsmock fixture. The dispatch glue is covered by the compile-driven signature change; the pureproject()half gets a unit test, and the e2emechanical_reactionscenario now asserts a delivered fire's record carries a payload (that gate fails ifpayload: Some(&delivered)is ever reverted).Compatibility
Additive and optional (
skip_serializing_if). Records written before this field deserialize unchanged and render with no pane — the console gates ont.payload !== undefined, so a legitimately falsy delivered payload (null,0,false,"") still renders.Testing
cargo test -p harness --lib— 303 passed (baseline 302 + 1)cargo fmt --check,cargo clippy --all-features -- -D warnings— cleanconsole/web: 87 files / 1121 tests passed;tsc --noEmitclean; biome clean on touched filesmechanical_reactioncompiles and passes its offline suite; the live scenario needsIII_BINand runs in CIKnown / deferred
JSON.stringify+ Prism tokenize per card, no memoization or virtualization. This is the existing console-wide pattern (same asRegistrationPane,SpawnView, the scrapling views) — widened here, not invented. Deferred.Fixes MOT-4365.
Summary by CodeRabbit