Add trajectory reviewer - #80
Conversation
- `view_trajectories()` → `trajectory_review()` - `read_trajectory_reviews()` → `trajectory_reviews_read()` - Update documentation and examples accordingly
simonpcouch
left a comment
simonpcouch
left a comment
There was a problem hiding this comment.
Super excited about this! Some initial thoughts here.
| #' @return A list of actionable review records in file order: all notes and | ||
| #' currently active flags. When `trajectories` is supplied, each record also | ||
| #' contains `turns`. | ||
| #' @export |
There was a problem hiding this comment.
Part of me wonders whether we ought to write the reviews with the turns 'joined' in in the first place. Since it's a human that reviews the responses, I would guess that the scale of these wouldn't reach past 100s of comments, so these files wouldn't ultimately end up huge.
The reason I say this is because I would suspect that humans would not be the ones to then go and read in all of the reviews + process them, so I'd prefer we not export a function mostly for the coding agent's usage. Instead, that format that contains both the turns and the reviews could be document with a few-liner in the trajectory_review() docs that gets the agent most of the way there.
There was a problem hiding this comment.
Garrick thinks we should probably have a function like this. He also suggested maybe this ends up being a directory of markdown files so that they're super agent-readable (whether that's immediately what they become or get post-processed into after this intermediate state)
There was a problem hiding this comment.
I like Garrick's idea, and I think regardless of format, the review writing and ingesting process needs a second look. For now:
- I removed
trajectory_reviews_read()for the reason you stated, and because the trajectory review format may change. It's not needed to read in the trajectories, so this doesn't result in any loss of functionality. - I did not change the review
jsonlformat to include the turns because the format may change and I am also a bit hesitant to duplicate that data (but could definitely be convinced that this isn't a problem). - I can plan to revisit the review log format for v2.
How does that sound?
|
Feedback from Paul & Garrick:
|
|
@simonpcouch thank you!
I think we can defer the changes that came out of the feedback from Paul and Garrick to a follow up PR. Specifically, I think it's worth revisiting how review data is stored and possibly using markdown like Garrick suggested. |
|
Preview deployed to Connect ( Deployed from commit 3b524d1. |
|
Preview deployed to Connect ( Deployed from commit 3b524d1. |
simonpcouch
left a comment
simonpcouch
left a comment
There was a problem hiding this comment.
Great, thanks so much! Agreed that we can iterate on the review reading and other UI points after merge.
In addition to merging upstream, I went ahead and did the read_trajectories() -> trajectory_read() rename, so the function names are consistent, and added you as author.
|
Cleaned up 2 preview bundle(s) on https://dogfood.team.pct.posit.it: 358569, 358570 |
|
Cleaned up 1 preview bundle(s) on https://connect.staging.pct.posit.it: 1658 |
2 participants
Adds a trajectory reviewer app.
Launch the reviewer app with:
Trust tags are reconstructed from the conversation, see #79.
Notes and flags are written to an append-only JSONL file. There is probably later work to do on how the review data is stored. You can join the review data back to the trajectories data for analysis:
Addresses #53