Implement spec 33 pipeline vocabulary patterns#7
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AppConfig as AppConfig
participant TranscriptCfg as TranscriptConfig
participant Resolver as ResolvedBuiltinStepConfig
participant Refine as Refine Module
User->>AppConfig: Load config (may include system_prompt_append)
AppConfig->>TranscriptCfg: Validate system_prompt and system_prompt_append
TranscriptCfg-->>AppConfig: Validation result
AppConfig->>Resolver: Build ResolvedBuiltinStepConfig
Resolver->>TranscriptCfg: materialize_system_prompt()
TranscriptCfg->>TranscriptCfg: Compose base system_prompt + system_prompt_append
TranscriptCfg-->>Resolver: Return composed system_prompt (append cleared)
Resolver-->>AppConfig: Resolved config ready
AppConfig->>Refine: Provide composed system_prompt
Refine->>Refine: build_refine_user_prompt(hint_prompt, raw_text)
Refine-->>User: Send refine request with preserved vocabulary JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
|
@Auggie review |
![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: 94446eb185
ℹ️ 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 let Some(system_prompt) = self.system_prompt.as_ref() { | ||
| transcript.system_prompt = system_prompt.clone(); | ||
| } | ||
| if let Some(system_prompt_append) = self.system_prompt_append.as_deref() { | ||
| transcript.append_system_prompt(system_prompt_append); |
There was a problem hiding this comment.
When a later voice/profile switches to a different system_prompt, any previously accumulated system_prompt_append text is left intact until materialize_system_prompt() runs. In configs that set a global vocabulary JSON block and then use a contextual system_prompt as a full replacement, the old append block still gets sent to refine, so a mail/profile-specific prompt can unexpectedly inherit developer-only vocabulary hints from the base config. The same replacement path in TranscriptOverrides::apply_to below has this behavior too.
Useful? React with 👍 / 👎.
|
|
||
| impl TranscriptConfig { | ||
| fn validate(&self, field_prefix: &str) -> Result<(), ConfigValidationError> { | ||
| validate_prompt_fragment(&self.system_prompt, format!("{field_prefix}.system_prompt"))?; |
There was a problem hiding this comment.
This now treats [transcript].system_prompt = "" as a validation error, even though the refine step already has its own built-in system prompt and previously accepted an empty per-project hint. Existing configs that intentionally blank the hint prompt to opt out of extra guidance will now fail AppConfig::load(); for the standalone refine tool, load_refine_config_from_config() will even discard the whole config and fall back to defaults. That is a backward-compatibility regression unrelated to the new append helper.
Useful? React with 👍 / 👎.
🤖 Augment PR SummarySummary: This PR implements the “spec 33” pipeline vocabulary pattern by letting users layer bounded vocabulary-hint blocks into the existing refine prompt surface. Changes:
Technical Notes: Muninn forwards appended blocks as plain text to refine (best-effort prompt biasing); it does not parse JSON or integrate provider-native vocabulary/adaptation APIs. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| impl TranscriptConfig { | ||
| fn validate(&self, field_prefix: &str) -> Result<(), ConfigValidationError> { | ||
| validate_prompt_fragment(&self.system_prompt, format!("{field_prefix}.system_prompt"))?; |
There was a problem hiding this comment.
TranscriptConfig::validate now rejects an empty transcript.system_prompt, which seems like it could prevent intentionally disabling refine hints (or using only system_prompt_append), even though other code paths treat an empty prompt as meaningful (e.g., skipping refine context). Was tightening this validation intended to be a breaking config change?
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.rs (1)
721-727:⚠️ Potential issue | 🟠 MajorClear inherited append fragments when
system_promptis replaced.Line 723 and Line 898 overwrite
system_promptbut keep any previously accumulatedsystem_prompt_append. That means a voice/profile “full replacement” still inherits earlier vocabulary blocks, so the final refine prompt can include stale hints from the base config or prior voice layer.Suggested fix
impl TranscriptConfig { + fn replace_system_prompt(&mut self, prompt: &str) { + self.system_prompt = prompt.to_string(); + self.system_prompt_append = None; + } + fn append_system_prompt(&mut self, fragment: &str) { append_prompt_fragment(&mut self.system_prompt_append, fragment); } } @@ fn apply_to(&self, transcript: &mut TranscriptConfig, refine: &mut RefineConfig) { if let Some(system_prompt) = self.system_prompt.as_ref() { - transcript.system_prompt = system_prompt.clone(); + transcript.replace_system_prompt(system_prompt); } if let Some(system_prompt_append) = self.system_prompt_append.as_deref() { transcript.append_system_prompt(system_prompt_append); } @@ fn apply_to(&self, transcript: &mut TranscriptConfig) { if let Some(system_prompt) = self.system_prompt.as_ref() { - transcript.system_prompt = system_prompt.clone(); + transcript.replace_system_prompt(system_prompt); } if let Some(system_prompt_append) = self.system_prompt_append.as_deref() { transcript.append_system_prompt(system_prompt_append); } }Please add a regression test for “base append + overriding
system_prompt” at both the voice and profile layers.Also applies to: 896-902
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 721 - 727, When replacing system_prompt in apply_to, clear any accumulated append fragments so a full replacement does not inherit prior appends: in apply_to (the block that checks self.system_prompt.as_ref()) after setting transcript.system_prompt = system_prompt.clone(), reset the transcript's system-prompt-append storage (e.g., clear or set to empty the field used by transcript.append_system_prompt); do the same for the analogous replacement block that handles RefineConfig/profile (the other block referenced around lines 896–902). Also add regression tests that create a base config with system_prompt_append, then apply a voice-layer and a profile-layer config that sets system_prompt (full replacement) and assert that no prior append fragments remain in the final refine/system prompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.rs`:
- Around line 721-727: When replacing system_prompt in apply_to, clear any
accumulated append fragments so a full replacement does not inherit prior
appends: in apply_to (the block that checks self.system_prompt.as_ref()) after
setting transcript.system_prompt = system_prompt.clone(), reset the transcript's
system-prompt-append storage (e.g., clear or set to empty the field used by
transcript.append_system_prompt); do the same for the analogous replacement
block that handles RefineConfig/profile (the other block referenced around lines
896–902). Also add regression tests that create a base config with
system_prompt_append, then apply a voice-layer and a profile-layer config that
sets system_prompt (full replacement) and assert that no prior append fragments
remain in the final refine/system prompt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b0aaa57-6ba7-4967-afc4-e0bf5922bc3c
📒 Files selected for processing (7)
README.mdbenches/runtime_bottlenecks.rsconfigs/config.sample.tomlspecs/33-pipeline-vocabulary-patterns/tasks.mdsrc/config.rssrc/refine.rssrc/stt_deepgram_tool.rs
1 participant
Summary
Validation
Summary by CodeRabbit
New Features
Documentation