Implement build_edit_prompt for history editing (issue #99)
- Add HistoryEditor::build_edit_prompt() to format edit prompts with previous message context and >8 separator - Message 0 shows '(no previous message)' instead of context - Add -T debug flag to test build_edit_prompt - Extend mock_editor.sh to support sequential invocations via counter file and staged input files - Add unit tests for build_edit_prompt functionality - Register new test in CMakeLists.txt
This commit is contained in:
@@ -32,6 +32,13 @@ public:
|
||||
// Empty messages display as: (empty)
|
||||
std::string build_rebase_content(const std::vector<Message>& history) const;
|
||||
|
||||
// Build the edit prompt for editing a single message
|
||||
// Shows previous message as context, separator, and current content to edit
|
||||
// For message 0, shows "(no previous message)" instead of previous message
|
||||
std::string build_edit_prompt(const std::vector<Message>& history,
|
||||
size_t edit_index,
|
||||
const std::string& role) const;
|
||||
|
||||
private:
|
||||
// Extract the first line of content from a message
|
||||
std::string get_first_line(const std::string& content) const;
|
||||
|
||||
@@ -95,6 +95,9 @@ int main(int argc, char** argv) {
|
||||
char const* test_build_rebase_json = nullptr;
|
||||
char const* test_apply_rebase_lines = nullptr;
|
||||
char const* test_apply_rebase_history = nullptr;
|
||||
char const* test_edit_prompt_json = nullptr;
|
||||
char const* test_edit_prompt_index = nullptr;
|
||||
char const* test_edit_prompt_role = nullptr;
|
||||
|
||||
ToolRegistry master_registery;
|
||||
ToolRegistry agent_tools;
|
||||
@@ -226,6 +229,13 @@ int main(int argc, char** argv) {
|
||||
test_apply_rebase_lines = consume_arg();
|
||||
test_apply_rebase_history = consume_arg();
|
||||
}
|
||||
else if (argv[0] == std::string("-T"))
|
||||
{
|
||||
consume_arg();
|
||||
test_edit_prompt_json = consume_arg();
|
||||
test_edit_prompt_index = consume_arg();
|
||||
test_edit_prompt_role = consume_arg();
|
||||
}
|
||||
else if (argv[0] == std::string("-L"))
|
||||
{
|
||||
consume_arg();
|
||||
@@ -280,6 +290,7 @@ int main(int argc, char** argv) {
|
||||
<< " -P <size> <content> Debug: test rebase parser and exit\n"
|
||||
<< " -B <json> Debug: test build_rebase_content with JSON history and exit\n"
|
||||
<< " -A <lines> <history> Debug: test apply_rebase with JSON and exit\n"
|
||||
<< " -T <json> <index> <role> Debug: test build_edit_prompt and exit\n"
|
||||
<< " -d Use debug client (mock API via stdin)\n"
|
||||
<< " -h Display this help menu\n";
|
||||
exit(0);
|
||||
@@ -565,6 +576,37 @@ int main(int argc, char** argv) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (test_edit_prompt_json && test_edit_prompt_index && test_edit_prompt_role) {
|
||||
// Parse history JSON
|
||||
auto j = parse_json_safely(test_edit_prompt_json);
|
||||
if (j.is_discarded()) {
|
||||
std::cout << "error: invalid JSON" << std::endl;
|
||||
return 1;
|
||||
}
|
||||
|
||||
auto history_result = json_to<std::vector<Message>>(j);
|
||||
if (!history_result) {
|
||||
std::cout << "error: " << history_result.error() << std::endl;
|
||||
return 1;
|
||||
}
|
||||
|
||||
// Parse index
|
||||
size_t edit_index;
|
||||
try {
|
||||
edit_index = std::stoul(test_edit_prompt_index);
|
||||
} catch (const std::exception&) {
|
||||
std::cout << "error: invalid edit index: " << test_edit_prompt_index << std::endl;
|
||||
return 1;
|
||||
}
|
||||
|
||||
const std::string role = test_edit_prompt_role;
|
||||
|
||||
HistoryEditor editor;
|
||||
std::string prompt = editor.build_edit_prompt(history_result.value(), edit_index, role);
|
||||
std::cout << prompt;
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (list_models)
|
||||
{
|
||||
auto models_result = create_client(use_debug)->list_models();
|
||||
|
||||
@@ -194,6 +194,32 @@ std::string HistoryEditor::build_rebase_content(const std::vector<Message>& hist
|
||||
return oss.str();
|
||||
}
|
||||
|
||||
std::string HistoryEditor::build_edit_prompt(const std::vector<Message>& history,
|
||||
size_t edit_index,
|
||||
const std::string& role) const {
|
||||
std::ostringstream oss;
|
||||
|
||||
// Show previous message as context
|
||||
if (edit_index == 0) {
|
||||
oss << "# (no previous message)\n";
|
||||
} else {
|
||||
const auto& prev_msg = history[edit_index - 1];
|
||||
oss << "# Previous message:\n\n";
|
||||
oss << "[" << (edit_index - 1) << "] " << prev_msg.role << ": " << prev_msg.content << "\n";
|
||||
}
|
||||
|
||||
// Separator
|
||||
oss << "\n-- >8 --\n";
|
||||
oss << "Edit message " << edit_index << " (role: " << role << ") below:\n\n";
|
||||
|
||||
// Current content (editable)
|
||||
if (edit_index < history.size()) {
|
||||
oss << history[edit_index].content;
|
||||
}
|
||||
|
||||
return oss.str();
|
||||
}
|
||||
|
||||
std::expected<std::vector<Message>, std::string> RebaseEngine::apply_rebase(
|
||||
const std::vector<RebaseLine>& lines,
|
||||
const std::vector<Message>& history) const {
|
||||
|
||||
@@ -60,5 +60,8 @@ add_dr_test(test_rebase_content)
|
||||
# Add rebase apply tests
|
||||
add_dr_test(test_rebase_apply)
|
||||
|
||||
# Add edit prompt tests
|
||||
add_dr_test(test_edit_prompt)
|
||||
|
||||
# Add edit history flag tests
|
||||
add_dr_test(test_edit_history_flag)
|
||||
|
||||
@@ -2,6 +2,33 @@
|
||||
# $1 is the file path passed to the editor
|
||||
FILE=$1
|
||||
|
||||
# Support sequential invocations using a counter file
|
||||
# Track invocations in $TEST_TEMP_DIR/.editor_invocation_count
|
||||
# Copy from $TEST_TEMP_DIR/.editor_input_$INVOCATION to the editor file
|
||||
if [ -n "$TEST_TEMP_DIR" ] && [ -f "$TEST_TEMP_DIR/.editor_input_0" ]; then
|
||||
# Initialize counter file if it doesn't exist
|
||||
if [ ! -f "$TEST_TEMP_DIR/.editor_invocation_count" ]; then
|
||||
echo "0" > "$TEST_TEMP_DIR/.editor_invocation_count"
|
||||
fi
|
||||
|
||||
# Read current invocation count
|
||||
INVOCATION=$(cat "$TEST_TEMP_DIR/.editor_invocation_count")
|
||||
|
||||
# Check if there's a staged input file for this invocation
|
||||
STAGED_INPUT="$TEST_TEMP_DIR/.editor_input_$INVOCATION"
|
||||
if [ -f "$STAGED_INPUT" ]; then
|
||||
# Copy staged content to the editor file
|
||||
cp "$STAGED_INPUT" "$FILE"
|
||||
fi
|
||||
|
||||
# Increment the counter for next invocation
|
||||
echo $((INVOCATION + 1)) > "$TEST_TEMP_DIR/.editor_invocation_count"
|
||||
|
||||
# For inspecting by tests
|
||||
cat $FILE > hist.md
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# For inspecting by tests
|
||||
cat $FILE > hist.md
|
||||
|
||||
|
||||
@@ -0,0 +1,206 @@
|
||||
#!/bin/bash
|
||||
# test_edit_prompt.sh - Tests for HistoryEditor::build_edit_prompt()
|
||||
|
||||
# Test 1: Edit message with previous message context
|
||||
test_edit_with_context() {
|
||||
local test_name="edit_with_context"
|
||||
|
||||
# Two messages: editing message 1, so message 0 should be shown as context
|
||||
local json='[{"role":"user","content":"hello"},{"role":"assistant","content":"hi there"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "1" "assistant")
|
||||
|
||||
# Check for previous message context
|
||||
if ! echo "$OUTPUT" | grep -q "# Previous message:"; then
|
||||
echo "FAIL: $test_name - missing previous message header"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for index 0 in context
|
||||
if ! echo "$OUTPUT" | grep -q "\[0\] user: hello"; then
|
||||
echo "FAIL: $test_name - missing context message with index"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for separator
|
||||
if ! echo "$OUTPUT" | grep -q -- "-- >8 --"; then
|
||||
echo "FAIL: $test_name - missing separator"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for edit prompt
|
||||
if ! echo "$OUTPUT" | grep -q "Edit message 1 (role: assistant) below:"; then
|
||||
echo "FAIL: $test_name - missing edit prompt"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for current content
|
||||
if ! echo "$OUTPUT" | grep -q "hi there"; then
|
||||
echo "FAIL: $test_name - missing current content"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Test 2: Edit first message (no previous message)
|
||||
test_edit_first_message() {
|
||||
local test_name="edit_first_message"
|
||||
|
||||
local json='[{"role":"user","content":"hello"},{"role":"assistant","content":"hi there"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "0" "user")
|
||||
|
||||
# Check for "no previous message" text
|
||||
if ! echo "$OUTPUT" | grep -q "# (no previous message)"; then
|
||||
echo "FAIL: $test_name - should show no previous message"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Should not have "Previous message:" header
|
||||
if echo "$OUTPUT" | grep -q "# Previous message:"; then
|
||||
echo "FAIL: $test_name - should not show previous message header for index 0"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for separator
|
||||
if ! echo "$OUTPUT" | grep -q -- "-- >8 --"; then
|
||||
echo "FAIL: $test_name - missing separator"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for edit prompt
|
||||
if ! echo "$OUTPUT" | grep -q "Edit message 0 (role: user) below:"; then
|
||||
echo "FAIL: $test_name - missing edit prompt"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Check for current content
|
||||
if ! echo "$OUTPUT" | grep -q "hello"; then
|
||||
echo "FAIL: $test_name - missing current content"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Test 3: Separator correctly placed
|
||||
test_separator_placement() {
|
||||
local test_name="separator_placement"
|
||||
|
||||
local json='[{"role":"user","content":"first"},{"role":"assistant","content":"second"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "1" "assistant")
|
||||
|
||||
# The separator should appear after the context and before the edit prompt
|
||||
# Check that ">8 --" is followed by the edit message prompt
|
||||
if ! echo "$OUTPUT" | grep -A2 -- "-- >8 --" | grep -q "Edit message 1"; then
|
||||
echo "FAIL: $test_name - separator not correctly placed before edit prompt"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Test 4: Multi-line content in context shows full content
|
||||
test_multiline_context() {
|
||||
local test_name="multiline_context"
|
||||
|
||||
local json='[{"role":"user","content":"line one\nline two"},{"role":"assistant","content":"response"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "1" "assistant")
|
||||
|
||||
# Both lines should appear in context
|
||||
if ! echo "$OUTPUT" | grep -q "line one"; then
|
||||
echo "FAIL: $test_name - first line of context missing"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
if ! echo "$OUTPUT" | grep -q "line two"; then
|
||||
echo "FAIL: $test_name - second line of context missing"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Test 5: Edit with tool role
|
||||
test_edit_tool_role() {
|
||||
local test_name="edit_tool_role"
|
||||
|
||||
local json='[{"role":"assistant","content":"call tool"},{"role":"tool","content":"result"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "1" "tool")
|
||||
|
||||
# Check for tool role in edit prompt
|
||||
if ! echo "$OUTPUT" | grep -q "Edit message 1 (role: tool) below:"; then
|
||||
echo "FAIL: $test_name - should show tool role in edit prompt"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Test 6: Edit middle message shows only immediate previous as context
|
||||
test_edit_middle_message() {
|
||||
local test_name="edit_middle_message"
|
||||
|
||||
local json='[{"role":"user","content":"msg0"},{"role":"assistant","content":"msg1"},{"role":"tool","content":"msg2"}]'
|
||||
|
||||
OUTPUT=$("${DRHARNESS}" -T "$json" "2" "tool")
|
||||
|
||||
# Should show message 1 as context (immediate previous)
|
||||
if ! echo "$OUTPUT" | grep -q "\[1\] assistant: msg1"; then
|
||||
echo "FAIL: $test_name - should show message 1 as context"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Should NOT show message 0 in context
|
||||
if echo "$OUTPUT" | grep -q "\[0\]"; then
|
||||
echo "FAIL: $test_name - should not show message 0 in context"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Should show edit prompt for message 2
|
||||
if ! echo "$OUTPUT" | grep -q "Edit message 2 (role: tool) below:"; then
|
||||
echo "FAIL: $test_name - should show edit prompt for message 2"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Should show current content (msg2)
|
||||
if ! echo "$OUTPUT" | grep -q "msg2"; then
|
||||
echo "FAIL: $test_name - should show current content"
|
||||
echo "Got: $OUTPUT"
|
||||
return 1
|
||||
fi
|
||||
|
||||
echo "PASS: $test_name"
|
||||
}
|
||||
|
||||
# Run all tests
|
||||
test_edit_with_context
|
||||
test_edit_first_message
|
||||
test_separator_placement
|
||||
test_multiline_context
|
||||
test_edit_tool_role
|
||||
test_edit_middle_message
|
||||
|
||||
echo ""
|
||||
echo "All tests completed."
|
||||
Reference in New Issue
Block a user