Split the monolithic test_edit_history.sh file into 21 individual
test scripts, each containing a single test. This follows the
project's testing guideline of having no more than one test per script.
Each test file contains its own helper functions (run_drharness,
get_output, reset_editor_state) and runs independently.
Updated CMakeLists.txt to register all 21 new test targets and
removed the old test_edit_history target.
- Modified parse_rebase_file to skip header comments before '-- >8 --'
- Modified apply_rebase to drop messages whose indices are not mentioned
in the parsed commands (implicit drop)
- Special case: if no commands are parsed (all lines deleted), keep all messages
- Updated test_implicit_pick to reflect new behavior
- Added test_deleting_lines_drops_messages and test_all_lines_deleted_keeps_all
- Updated test_delete_line_equals_drop to reflect new behavior
Add string_toul() function in utility.cpp that wraps std::stoul and returns
std::expected<unsigned long, std::string> instead of throwing exceptions.
Replace all uses of std::stoul with string_toul():
- src/main.cpp: 2 occurrences in test flags
- src/rebase_engine.cpp: 1 occurrence in parse_rebase_file()
The wrapper handles null input, empty input, and trailing characters,
converting any exceptions from std::stoul into error strings.
- Added format_error() helper function to centralize line number error formatting
- Replaced all 11 occurrences of inline line number formatting with calls to format_error()
- No functional changes - all 33 tests pass
- Removed the errors vector from parse_rebase_file()
- Changed all error handling to return std::unexpected() immediately
- Removed the error aggregation code at the end of the function
- Updated test to verify first error is returned (not all errors)
This refactors the duplicated code for deserializing HarnessState from a file
into a reusable helper function. The same pattern was used in two places:
1. In the edit_history block
2. In the main flow when loading existing chat files
The helper function handles:
- Opening the file
- Reading file content
- Parsing JSON
- Converting to HarnessState
- Error reporting
This reduces code duplication and makes future modifications easier.
As per issue #115, moved the History Editing documentation from README.md
to a separate docs/history-editing.md file. Added a short note with a link
to the new documentation in README.md.
- Add test for editing a message and removing all content (empty content)
- Add test for multiple consecutive edits to the same message (changes stack)
- Add test for editing messages with special characters
- Add test for very long message content
- Add test for editing assistant messages with tool_calls
- Add test for dropping all messages (empty history result)
- Add test for unicode characters in message content
- Add test for whitespace variations in rebase file
- Add test for comments and empty lines mixed with commands
- Add test for special characters and unicode in rebase content display
- Fix apply_rebase to preserve original content when only role is changed
- Add content_edited flag to RebaseLine to track explicit edits
- Changed parse_json_safely to return std::expected<nlohmann::json, std::string>
- Updated all usages to handle the expected return type properly
- Simplified test flags in main.cpp to be minimal hooks without extra validation
- Removed try/catch blocks and explicit error handling from test flags
- All tests pass
The -J flag was outputting 'valid' or 'discarded' instead of the raw
JSON result. This was test logic that should be in the test scripts.
Changed the -J flag to output the raw JSON result (pretty-printed) or
an error message. Updated the test script to use jq for JSON validation.
All other debug flags (-E, -e, -R, -P, -B, -A, -T) were already minimal
hooks and didn't need to be changed.
Per CONTRIBUTING.md, test flags should be minimal hooks that:
- Accept input, call the internal function, and output the result
- Do NOT contain test logic (assertions, validation, etc.)
Changes:
1. Added json_to<RebaseCommand> and json_to<RebaseLine> serialization
so the -A flag can use json_to<std::vector<RebaseLine>> instead of
60+ lines of manual JSON parsing.
2. Simplified -P flag to output JSON directly instead of formatted
text with switch statements.
3. Added helper functions to run_test.sh for parsing rebase JSON output
(rebase_line_count, rebase_command_at, rebase_index_at, rebase_role_at).
4. Updated test_rebase_parser.sh to use JSON output and helper functions.
The -E, -e, -R, -J, -B, and -T flags are already minimal hooks and
remain unchanged.
Clarify that test flags should be minimal hooks into internal
functionality, not contain test logic. All test logic (assertions,
validation) belongs in Bash test scripts in tests/.
This addresses the ambiguity mentioned in issue 109 regarding
the distinction between test hooks in main.cpp and test scripts.
Add create_chat_file() helper to run_test.sh and update test_edit_history.sh
and test_edit_history_flag.sh to use it instead of manually creating JSON.
This follows the CONTRIBUTING.md guideline that tests should use helper
functions instead of inline JSON handling.
- Add dr_reasoning() helper to run_test.sh
- Add dr_reasoning_content() helper to run_test.sh
- Update process_reasoning.sh to use helper functions instead of direct jq calls
This follows CONTRIBUTING.md guidelines that prohibit direct JSON
parsing in test files.
The edit prompt now shows the explanatory text 'Edit message X (role: Y) below:'
BEFORE the scissors (-- >8 --) separator, not after. This ensures the
explanatory text is not part of the editable content.
Also added test_edit_prompt_noop to verify that doing nothing at the edit
prompt stage keeps history unchanged.
- Added new test test_no_edits_history_unchanged that verifies when the
user does nothing with the rebase editor, the history remains unchanged
- Fixed staged input format in all tests to match actual build_rebase_content
output (pick commands only after -- >8 -- scissors line, not before)
The rebase editor now includes a scissors line (-- >8 --) that separates
the comment header from the actual pick commands. This helps editors like
Vim automatically position the cursor at the relevant content and signals
to users where the editable commands begin.
Issue: #104
Create new utility module (include/utility.hpp, src/utility.cpp) containing
common code that was duplicated between rebase_engine.cpp and editor.cpp:
- trim() function
- random_string() function
- random_file_name() function
- Defer RAII helper class
- spawn_editor() function
Both rebase_engine.cpp and editor.cpp now include utility.hpp instead of
duplicating these functions.
All 33 unit tests pass.
- Remove namespace alias in src/rebase_engine.cpp (use full std::filesystem:: path)
- Add dr_role() helper function to tests/run_test.sh
- Replace inline JSON parsing in tests/test_edit_history.sh with dr_role() helper
- Add edit_history_interactive() method to HistoryEditor class
- Implements the complete history editing flow:
1. Build rebase content from history
2. Spawn editor for rebase commands
3. Parse and validate rebase commands
4. For each edit command, build edit prompt and spawn editor
5. Apply all changes
- Returns expected<vector<Message>, string>
- Add content field to RebaseLine struct for storing edited content
- Update apply_rebase() to use content field when present
- Wire up -b/--edit-history flag to call edit_history_interactive()
- Program exits after successful edit and saves modified history
- Add 12 integration tests for the edit history feature:
1. Pick all messages (history unchanged)
2. Drop a single message
3. Edit message content
4. Change message role
5. Delete line equals drop (implicit pick)
6. Edit first message (no context)
7. Empty history rejected
8. Invalid command error
9. Invalid index error
10. Invalid role error
11. Multiple edits in one session
12. Edit then continue conversation normally
- Register test_edit_history in CMakeLists.txt
- 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
- Replace .at("field").get<T>() with json_to_field<T>() calls
- Use .contains() and .value() for safe field access
- Follows CONTRIBUTING.md rule to always use json_to<T>() for JSON deserialization
- Added apply_rebase() method to RebaseEngine class
- Pick commands preserve messages unchanged
- Drop commands remove messages from result
- Edit commands update role while preserving all other fields
- Messages not referenced by any command are preserved (implicit pick)
- Lenient mode: orphaned tool results are allowed
- Added -A test flag for testing apply_rebase
- Added unit tests in test_rebase_apply.sh
Add the -b/--edit-history CLI flag to main.cpp with the following behavior:
- Flag is recognized in both short (-b) and long (--edit-history) forms
- Requires -F <chat_file> to be specified, otherwise prints error and exits
- Rejects empty chat files with error message
- Rejects chat files with no history (empty history array) with error message
- Updates help message to document the new flag
This is a thin slice that validates the CLI integration point before the
full history editing feature is built.
Includes integration tests covering all error cases.
- Added HistoryEditor class with build_rebase_content() method
- Formats messages for rebase editor display:
- Normal messages show first line of content
- Assistant messages with tool_calls show [tool_call: function_name]
- Tool messages show first line of result content
- Empty messages show (empty)
- Added header comment block explaining pick/edit/drop commands
- Added -B CLI flag for testing
- Added unit tests covering all display format scenarios
Add rebase parser that validates rebase editor format with:
- RebaseCommand enum (Pick, Edit, Drop)
- RebaseLine struct (command, index, role)
- parse_rebase_file() with strict validation and error aggregation
Validations:
- Invalid commands (not pick/edit/drop)
- Index out of range
- Duplicate indices
- Missing/malformed role brackets
- Invalid role values (must be user/assistant/tool)
- Comment lines (#) and empty lines are skipped
Tests cover all validation scenarios via -P debug flag.
Replace std::any-based parameter passing with compile-time type-safe adapter pattern.
- Add create_tool_execute<T>() template function in tool.hpp
- Remove convert_params field from Tool struct
- Change execute field type from std::function<std::string(const std::any&)>
to std::function<std::string(const nlohmann::json&)>
- Change ToolRegistry::invoke() return type from std::expected to std::string
- Update all tool implementations to use create_tool_execute<ParamsType>()
- Remove #include <any> from tool.hpp and json_types.hpp
- Simplify harness.cpp invoke caller
All 27 tests pass.
Per CONTRIBUTING.md, inline JSON generation in tests should use helper
functions from run_test.sh. Added tool_call_raw() helper that generates
tool call responses without sanitizing the arguments (unlike tool_call()
which escapes JSON). Updated test_tool_invocation_failure to use the new
helper instead of inline JSON.
Closes issue 87
The original test provided 20 invalid JSON responses and checked for
'max retries exceeded'. However, the retry loop only attempts 19 calls
before hitting the limit, and providing fewer responses (e.g., 10) would
still pass because stdin exhaustion also eventually triggers the same
error message.
This fix:
1. Changes the response count from 20 to 19 (actual retry attempts)
2. Adds a check to ensure no 'failed to read from stdin' messages appear,
proving the test validates retry exhaustion rather than stdin exhaustion
3. Now if the count is reduced to 10, the test will correctly fail
Fixes issue 88
The previous test only checked exit code was non-zero but discarded all
output. Now the test captures stderr and verifies both the exit code and
that the correct error message was printed, making it a meaningful test of
expect_or_exit behavior.
Issue 86
1. Added try-catch in json_to_field for primitive types to catch
nlohmann::json::exception and convert to std::unexpected.
2. Added try-catch in json_to_impl::apply for primitive types to catch
nlohmann::json::exception and convert to std::unexpected.
3. Added test for deeply nested type error propagation:
Message → vector<ToolCall> → FunctionCall error propagation.
This ensures that type mismatches (e.g., string where int expected)
return descriptive error messages instead of crashing with exceptions.
Extract the duplicated has_json_to<Type> registration and json_dispatch_map
registration code from DRH_DEFINE_TYPE_REQUIRED, DRH_DEFINE_TYPE_OPTIONAL,
and DRH_DEFINE_TYPE_MIXED into two helper macros:
- DRH_JSON_REGISTER_HAS_JSON_TO(Type)
- DRH_JSON_REGISTER_DISPATCH(Type)
This eliminates code duplication while preserving identical functionality.
As per CONTRIBUTING.md, nlohmann::json::parse should not be called directly.
Replace all direct calls with the safe wrapper function parse_json_safely().
- debug_client.cpp: Use parse_json_safely for stdin input
- ollama_client.cpp: Use parse_json_safely for HTTP response parsing
- main.cpp: Read file content to string, then use parse_json_safely
- Replace hand-written json_to<HarnessState>() specialization with DRH_DEFINE_TYPE_MIXED macro
- Remove hand-written to_json() free function for HarnessState
- Remove explicit has_json_to<HarnessState> trait registration (now handled by macro)
- Update test to expect new error format that includes field path (more informative)
HarnessState now uses:
DRH_DEFINE_TYPE_MIXED(HarnessState, (history, tools), (config))
All 30 tests pass.
- Remove hand-written json_to<Choice>() specialization and to_json() function
- Add DRH_DEFINE_TYPE_MIXED(Choice, (message), (finish_reason)) macro call
- Update test expectations for nested error propagation to include field path prefix
(e.g., 'field message: missing required field: role' instead of 'missing required field: role')
This change improves consistency with the PRD goal of having all json_to<T>()
implementations use macros, and provides better error messages with field path
information for debugging.
Add DRH_DEFINE_TYPE_MIXED(Type, (required_fields...), (optional_fields...))
macro that generates to_json() and json_to<Type>() for structs with mixed
required and optional JSON fields.
Features:
- Required fields: checks presence, reads via json_to_field<T>() for proper
nested type error propagation
- Optional fields: checks contains() and !is_null() before reading; uses
struct member defaults otherwise
- Generates free to_json() that sets ALL fields (required and optional)
- Automatically registers has_json_to<Type>
- Supports empty field groups: (), () for all-optional/all-required types
- Uses __VA_OPT__ (C++20+) for empty group handling
Added test structs:
- MixedTypeSmokeTest: one required + one optional field
- MixedTypeAllRequiredTest: empty optional group
- MixedTypeAllOptionalTest: empty required group
Updated tests/test_json_type_mixed.sh with smoke tests and edge case tests.
darthrubik