Skip to content

feat: Include /context hooks and conversation summary in /context show --expand #1477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brandonskiser merged 13 commits into from
May 9, 2025
Merged

feat: Include /context hooks and conversation summary in /context show --expand #1477

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f413816
add hooks in /context
evanliu048 May 2, 2025
5277b3d
add summary in /context
evanliu048 May 2, 2025
78fa9df
include in --expand
evanliu048 May 2, 2025
da2a89f
update /context show --expand words
evanliu048 May 2, 2025
cf0d810
show available summary instead regenerate it
evanliu048 May 5, 2025
c2d8cb1
rename hooks
evanliu048 May 5, 2025
dcedc5a
revise
evanliu048 May 5, 2025
a6f1d56
Merge remote-tracking branch 'origin/main' into evanAddContext
evanliu048 May 6, 2025
3a96cec
cr
evanliu048 May 7, 2025
922b4d4
fmt
evanliu048 May 7, 2025
7939d19
cr
evanliu048 May 7, 2025
2ed035f
fix typo
evanliu048 May 7, 2025
b687a1f
Merge branch 'main' into evanAddContext
evanliu048 May 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/chat-cli/src/cli/chat/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ impl ContextSubcommand {
<em>help</em> <black!>Show an explanation for the context command</black!>

<em>show [--expand]</em> <black!>Display the context rule configuration and matched files</black!>
<black!>--expand: Print out each matched file's content</black!>
<black!>--expand: Print out each matched file's content, hook</black!>
<black!> configurations and last conversation summary </black!>

<em>add [--global] [--force] <<paths...>></em>
<black!>Add context rules (filenames or glob patterns)</black!>
Expand Down
4 changes: 4 additions & 0 deletions crates/chat-cli/src/cli/chat/conversation_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ impl ConversationState {
}
}

pub fn latest_summary(&self) -> Option<&str> {
self.latest_summary.as_deref()
}

pub fn history(&self) -> &VecDeque<(UserMessage, AssistantMessage)> {
&self.history
}
Expand Down
169 changes: 123 additions & 46 deletions crates/chat-cli/src/cli/chat/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,9 @@ impl ChatContext {
if let Some(context_manager) = &mut self.conversation_state.context_manager {
match subcommand {
command::ContextSubcommand::Show { expand } => {
fn map_chat_error(e: ErrReport) -> ChatError {
ChatError::Custom(e.to_string().into())
}
// Display global context
execute!(
self.output,
Expand Down Expand Up @@ -1611,6 +1614,28 @@ impl ChatContext {
}
}

if expand {
queue!(
self.output,
style::SetAttribute(Attribute::Bold),
style::SetForegroundColor(Color::DarkYellow),
style::Print("\n 🔧 Hooks:\n")
)?;
Self::print_hook_section(
&mut self.output,
&context_manager.global_config.hooks,
HookTrigger::ConversationStart,
)
.map_err(map_chat_error)?;

Self::print_hook_section(
&mut self.output,
&context_manager.global_config.hooks,
HookTrigger::PerPrompt,
)
.map_err(map_chat_error)?;
}

// Display profile context
execute!(
self.output,
Expand Down Expand Up @@ -1650,6 +1675,28 @@ impl ChatContext {
execute!(self.output, style::Print("\n"))?;
}

if expand {
queue!(
self.output,
style::SetAttribute(Attribute::Bold),
style::SetForegroundColor(Color::DarkYellow),
style::Print(" 🔧 Hooks:\n")
)?;
Self::print_hook_section(
&mut self.output,
&context_manager.profile_config.hooks,
HookTrigger::ConversationStart,
)
.map_err(map_chat_error)?;
Self::print_hook_section(
&mut self.output,
&context_manager.profile_config.hooks,
HookTrigger::PerPrompt,
)
.map_err(map_chat_error)?;
execute!(self.output, style::Print("\n"))?;
}

if global_context_files.is_empty() && profile_context_files.is_empty() {
execute!(
self.output,
Expand Down Expand Up @@ -1727,6 +1774,39 @@ impl ChatContext {
style::Print(format!("\nTotal: ~{} tokens\n\n", total_tokens)),
)?;

// Show last cached conversation summary if available, otherwise regenerate it
if expand {
if let Some(summary) =
self.conversation_state.latest_summary().map(|s| s.to_owned())
Copy link
Contributor

@brandonskiser brandonskiser May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside the else block of if global_context_files.is_empty() && profile_context_files.is_empty() {, so it won't ever get executed if both global and profile context matches nothing.

Also shouldn't be necessary to to_owned here, we should be able to just print a reference

Copy link
Contributor Author

@evanliu048 evanliu048 May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes total sense. Moved the logic out of the else block and ditched the unnecessary .to_owned(). Learned something new again — thanks for the sharp eyes!

{
let border = "═".repeat(self.terminal_width().min(80));
execute!(
self.output,
style::Print("\n"),
style::SetForegroundColor(Color::Cyan),
style::Print(&border),
style::Print("\n"),
style::SetAttribute(Attribute::Bold),
style::Print(" CONVERSATION SUMMARY"),
style::Print("\n"),
style::Print(&border),
style::SetAttribute(Attribute::Reset),
style::Print("\n\n"),
style::Print(&summary),
style::Print("\n\n")
)?;
} else {
self.compact_history(
Copy link
Contributor

@brandonskiser brandonskiser May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely don't do this - just check if there is a summary cached, and if so print it.

Copy link
Contributor Author

@evanliu048 evanliu048 May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌I’ve updated the code to check for a cached summary and print it if present, avoiding unnecessary regeneration.

Some(tool_uses.clone()),
pending_tool_index,
None,
true,
false,
)
.await?;
}
}

execute!(self.output, style::Print("\n"))?;
}
},
Expand Down Expand Up @@ -1978,48 +2058,6 @@ impl ChatContext {
},
}
} else {
fn print_hook_section(
output: &mut impl Write,
hooks: &HashMap<String, Hook>,
trigger: HookTrigger,
) -> Result<()> {
let section = match trigger {
HookTrigger::ConversationStart => "Conversation Start",
HookTrigger::PerPrompt => "Per Prompt",
};
let hooks: Vec<(&String, &Hook)> =
hooks.iter().filter(|(_, h)| h.trigger == trigger).collect();

queue!(
output,
style::SetForegroundColor(Color::Cyan),
style::Print(format!(" {section}:\n")),
style::SetForegroundColor(Color::Reset),
)?;

if hooks.is_empty() {
queue!(
output,
style::SetForegroundColor(Color::DarkGrey),
style::Print(" <none>\n"),
style::SetForegroundColor(Color::Reset)
)?;
} else {
for (name, hook) in hooks {
if hook.disabled {
queue!(
output,
style::SetForegroundColor(Color::DarkGrey),
style::Print(format!(" {} (disabled)\n", name)),
style::SetForegroundColor(Color::Reset)
)?;
} else {
queue!(output, style::Print(format!(" {}\n", name)),)?;
}
}
}
Ok(())
}
queue!(
self.output,
style::SetAttribute(Attribute::Bold),
Expand All @@ -2028,13 +2066,13 @@ impl ChatContext {
style::SetAttribute(Attribute::Reset),
)?;

print_hook_section(
Self::print_hook_section(
&mut self.output,
&context_manager.global_config.hooks,
HookTrigger::ConversationStart,
)
.map_err(map_chat_error)?;
print_hook_section(
Self::print_hook_section(
&mut self.output,
&context_manager.global_config.hooks,
HookTrigger::PerPrompt,
Expand All @@ -2049,13 +2087,13 @@ impl ChatContext {
style::SetAttribute(Attribute::Reset),
)?;

print_hook_section(
Self::print_hook_section(
&mut self.output,
&context_manager.profile_config.hooks,
HookTrigger::ConversationStart,
)
.map_err(map_chat_error)?;
print_hook_section(
Self::print_hook_section(
&mut self.output,
&context_manager.profile_config.hooks,
HookTrigger::PerPrompt,
Expand Down Expand Up @@ -2632,6 +2670,45 @@ impl ChatContext {
})
}

// Prints hook configuration grouped by trigger: conversation sesiion start or per user message
fn print_hook_section(output: &mut impl Write, hooks: &HashMap<String, Hook>, trigger: HookTrigger) -> Result<()> {
Copy link
Contributor

@brandonskiser brandonskiser May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should take a &mut self receiver instead, otherwise let's place it as a standalone function rather than an associated function

Also for doc comments, use /// instead of //

Copy link
Contributor Author

@evanliu048 evanliu048 May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! 🙏
You're right — since the method doesn't use any fields from self, I've moved it out as a standalone function and updated the doc comment to use ///.

let section = match trigger {
HookTrigger::ConversationStart => "On Session Start",
HookTrigger::PerPrompt => "Per User Message",
};
let hooks: Vec<(&String, &Hook)> = hooks.iter().filter(|(_, h)| h.trigger == trigger).collect();

queue!(
output,
style::SetForegroundColor(Color::Cyan),
style::Print(format!(" {section}:\n")),
style::SetForegroundColor(Color::Reset),
)?;

if hooks.is_empty() {
queue!(
output,
style::SetForegroundColor(Color::DarkGrey),
style::Print(" <none>\n"),
style::SetForegroundColor(Color::Reset)
)?;
} else {
for (name, hook) in hooks {
if hook.disabled {
queue!(
output,
style::SetForegroundColor(Color::DarkGrey),
style::Print(format!(" {} (disabled)\n", name)),
style::SetForegroundColor(Color::Reset)
)?;
} else {
queue!(output, style::Print(format!(" {}\n", name)),)?;
}
}
}
Ok(())
}

async fn tool_use_execute(&mut self, mut tool_uses: Vec<QueuedTool>) -> Result<ChatState, ChatError> {
// Verify tools have permissions.
for (index, tool) in tool_uses.iter_mut().enumerate() {
Expand Down
Loading