feat: add brainstorm prompt reframing expansion#44
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a hidden per-model prompt-variant expansion mode for brainstorm runs and replaces buffered Pi CLI stdout parsing with a streaming JSONL parser. It updates CLI args/dry-run estimates, persists prompt-variant metadata, rewires propose/evaluate to operate over provider+variant lineages, and adds tests/docs. ChangesPrompt Variant Expansion with Stream Parsing Fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements upstream prompt-variant expansion (L3) for brainstorm runs, introducing a hidden --prompt-variants CLI flag and expanding lineages to run models against strategically reframed prompts. It also refactors the Pi provider to stream-parse JSON event streams incrementally via a new spawn_cli_stream_lines helper, preventing out-of-memory issues from large stdout buffers. The review feedback suggests two valuable improvements: capping the accumulated stderr buffer in the streaming helper to avoid potential OOMs from subprocess stderr, and moving prompt-variant text generation outside of the provider loop to eliminate redundant allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let stderr_task = tokio::spawn(async move { | ||
| let mut reader = BufReader::new(stderr_handle); | ||
| let mut buf = String::new(); | ||
| while let Ok(n) = reader.read_line(&mut buf).await { | ||
| if n == 0 { | ||
| break; | ||
| } | ||
| } | ||
| buf | ||
| }); |
There was a problem hiding this comment.
To prevent potential out-of-memory (OOM) issues if a subprocess emits an extremely large volume of stderr, the stderr_task should cap the accumulated buffer size while continuing to drain the stream to avoid pipe-buffer deadlocks.
let stderr_task = tokio::spawn(async move {
let mut reader = BufReader::new(stderr_handle);
let mut buf = String::new();
let mut line = String::new();
while let Ok(n) = reader.read_line(&mut line).await {
if n == 0 {
break;
}
if buf.len() < MAX_STREAM_ERROR_OUTPUT_SIZE {
let remaining = MAX_STREAM_ERROR_OUTPUT_SIZE - buf.len();
buf.push_str(&line.chars().take(remaining).collect::<String>());
}
line.clear();
}
buf
});| let mut handles = tokio::task::JoinSet::new(); | ||
| for provider in providers { | ||
| let provider = provider.clone(); | ||
| let model_id = provider.model_id().clone(); | ||
| let sem = semaphore.clone(); | ||
| let prompt_text = prompts::brainstorm::brainstorm_prompt_variant_prompt(prompt); | ||
| let messages = vec![ | ||
| Message::system(brainstorm_system_prompt_for_strategy( | ||
| BrainstormIterationStrategy::ScoreOnly, | ||
| )), | ||
| Message::user(prompt_text), | ||
| ]; |
There was a problem hiding this comment.
Generating the prompt variant text is independent of the provider and can be done once outside the loop to avoid redundant string formatting and allocations.
| let mut handles = tokio::task::JoinSet::new(); | |
| for provider in providers { | |
| let provider = provider.clone(); | |
| let model_id = provider.model_id().clone(); | |
| let sem = semaphore.clone(); | |
| let prompt_text = prompts::brainstorm::brainstorm_prompt_variant_prompt(prompt); | |
| let messages = vec![ | |
| Message::system(brainstorm_system_prompt_for_strategy( | |
| BrainstormIterationStrategy::ScoreOnly, | |
| )), | |
| Message::user(prompt_text), | |
| ]; | |
| let mut handles = tokio::task::JoinSet::new(); | |
| let prompt_text = prompts::brainstorm::brainstorm_prompt_variant_prompt(prompt); | |
| for provider in providers { | |
| let provider = provider.clone(); | |
| let model_id = provider.model_id().clone(); | |
| let sem = semaphore.clone(); | |
| let messages = vec![ | |
| Message::system(brainstorm_system_prompt_for_strategy( | |
| BrainstormIterationStrategy::ScoreOnly, | |
| )), | |
| Message::user(prompt_text.clone()), | |
| ]; |
![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: 4b06b831d2
ℹ️ 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".
| .and_then(|delta| delta.as_str()) | ||
| { | ||
| delta_text.push_str(delta); | ||
| self.delta_text.push_str(delta); |
There was a problem hiding this comment.
In Pi streams where message_update events repeat the accumulated assistant content, this still appends every update into delta_text, so the Pi provider can again retain tens of MB (or more) of transport data even when a later message_end supplies the final text. That reintroduces the memory/large-stream failure mode this streaming parser is meant to avoid; store only the latest accumulated update or otherwise avoid accumulating repeated content.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?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: fc0fa7ec35
ℹ️ 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".
| if expanded { | ||
| ModelId::from_parts( | ||
| owner.provider(), | ||
| format!("{}+{variant_label}", owner.model()).replace('/', "_"), |
There was a problem hiding this comment.
When --prompt-variants per-model is used with two models from the same provider whose model names differ only by / versus _ (for example p/a/b and p/a_b, and ModelId explicitly allows slashes in the model portion), this normalization gives both lineages the same ModelId such as p/a_b+original. The proposal and score maps are keyed by that generated ID, so one lineage overwrites the other and the run silently drops/corrupts candidates; use a reversible escaping scheme or keep the real owner ID separate from an artifact-safe filename.
Useful? React with 👍 / 👎.
| } else if !self.delta_text.ends_with(delta) { | ||
| self.delta_text.push_str(delta); |
There was a problem hiding this comment.
If Pi emits true incremental deltas and two adjacent chunks have the same text (for example the answer haha streamed as "ha" then "ha"), this suffix check treats the second chunk as a duplicate and drops it, returning truncated content. The cumulative-update case is already handled by starts_with; for non-cumulative deltas, repeated chunks are valid output and should still be appended.
Useful? React with 👍 / 👎.
1 participant
Summary
brainstorm --prompt-variants off|per-modelVerification
cargo fmt --all -- --checkcargo check --workspacecargo test -p refinery_core brainstorm -qcargo test -p refinery_core prompts -qcargo test -p refinery_cli brainstorm -qcargo test --workspacecargo clippy -p refinery_core --all-targets -- -D warningscargo clippy -p refinery_cli --all-targets -- -D warningscargo clippy --workspace --all-targets -- -D warningsdocs/brainstorms/2026-06-04-brainstorm-l3-prompt-reframing-smoke.mdanddocs/brainstorms/2026-06-05-brainstorm-l3-updated-model-smoke.mdNotes
Production defaults are unchanged:
score-only,prompt-variants off,controversy_floor_7.