Repository navigation
fix(bt-daemon): report trace permalinks in session status - #230
Open
Wolfie (wolfiesch) wants to merge 1 commit into
Open
Wolfie (wolfiesch) wants to merge 1 commit into
Wolfie (wolfiesch) wants to merge 1 commit into
Conversation
`status.get` documents a per-route `permalink` that powers Pi's trace-link widget, but `BraintrustSink` never overrode `Sink::permalink`, so the field was always `null` and the widget never appeared.
The sink now records the root span id of the trace that most recently received a row and builds `{app_url}/app/{org}/p/{project}/logs?r={root}`, percent-encoding each segment the same way `bt` builds app URLs. Root rows use their span id as the row id, so `r` opens that trace. The consumer already publishes `sink.permalink()` after each successful flush, so the link appears once the first batch is delivered. A provisional root held for work has no link yet. The link stays `null` when no logs URL can be built: experiment and parent-span destinations, project logs addressed only by `project_id`, or a route without an org name. `protocol.md` now states these rules.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Author
|
CI passed on my fork at this PR's head ( |
This branch has not been deployed
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.
status.getdocuments a per-routepermalinkthat powers Pi's trace-link widget, butBraintrustSinknever overrodeSink::permalink, so the field was alwaysnulland the widget never appeared.The sink now records the root span id of the trace that most recently received a row and builds
{app_url}/app/{org}/p/{project}/logs?r={root}, percent-encoding each segment the same waybtbuilds app URLs. Root rows use their span id as the row id, soropens that trace. The consumer already publishessink.permalink()after each successful flush, so the link appears once the first batch is delivered. A provisional root held for work has no link yet. The link staysnullwhen no logs URL can be built: experiment and parent-span destinations, project logs addressed only byproject_id, or a route without an org name.protocol.mdnow states these rules.Pi refreshes its widget on session start, compaction, and tree navigation. A new session's first refresh runs before any trace is delivered, so its link appears at the next refresh. This PR leaves the plugin unchanged.
Validation (Rust 1.99.1, in
bt-daemon/):cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-featurestests/braintrust_sink.rscover a held root, encoding, and switching to a newer trace, plus the destinations without a link.btagainst this branch, traced a session through the Pi extension andbt trace daemon, and confirmedstatus.getreturned a permalink that opens that trace in the project logs.