feat(parity): close hashline and TUI slash-command parity gaps#154
Merged
feat(parity): close hashline and TUI slash-command parity gaps#154
Conversation
![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: 2af88581cd
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/uira-orchestration/src/tools/builtins/edit.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/builtins/edit.rs:87">
P1: Sequential multi-edit application with position-based LINE#ID refs is broken: after the first edit mutates the `lines` vec, subsequent edits' LINE#IDs reference stale positions. `verify_line_ref` will report a confusing "hashline mismatch" rather than the real cause (shifted indices). Consider either (a) applying edits in reverse order by position, (b) resolving all LINE#ID refs to indices *before* any mutations, or (c) documenting that only single-edit payloads (or position-less appends/prepends) are supported when using LINE#IDs.</violation>
</file>
<file name="crates/uira-orchestration/src/tools/comment_hook.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/comment_hook.rs:99">
P2: Guard hashline edit-line detection with the same supported-file check used by check_edit/check_write; otherwise unsupported files now trigger comment warnings.</violation>
</file>
<file name="crates/uira-orchestration/src/hooks/hooks/comment_checker.rs">
<violation number="1" location="crates/uira-orchestration/src/hooks/hooks/comment_checker.rs:82">
P2: This adds a second copy of the hashline line-parsing logic that already exists in tools/comment_hook.rs. Consider extracting the shared functionality into a common helper (or reusing the existing one) to avoid divergence and simplify maintenance.</violation>
</file>
<file name="crates/uira-orchestration/src/tools/builtins/read.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/builtins/read.rs:40">
P2: The per-line 2000-character truncation guard from the previous implementation was removed. A single very long line (common in minified JS/JSON or lockfiles) can now produce multi-megabyte tool output from a single `Read` call, causing excessive token consumption and potential context-limit failures. Restore the per-line truncation alongside hashline tagging.</violation>
</file>
<file name="crates/uira-orchestration/src/tools/builtins/hashline.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/builtins/hashline.rs:72">
P1: `right.trim_start()` strips ALL leading whitespace from the content portion, destroying indentation. For a line like `2#AB | return x`, this returns `"return x"` instead of `" return x"`. Since the hashline format uses `LINE#ID | content` (single space after `|`), only strip one leading space character to preserve the user's original indentation. This will corrupt indentation-sensitive files (Python, YAML, etc.).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/uira-orchestration/src/tools/comment_shared.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/comment_shared.rs:47">
P3: Duplicate hashline parsing logic; `parse_edit_line_content` mirrors `hashline::parse_line_content`, which makes future format changes easy to miss. Consider reusing `hashline::parse_line_content` (and its `parse_line_ref`) to keep behavior in one place.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/uira-orchestration/src/tools/comment_shared.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/comment_shared.rs:27">
P3: parse_edit_line_content now strips prefixes for invalid hashline tags like "0#AB | ..." because parse_line_content treats line 0 as valid. This can remove legitimate content during comment detection. Consider rejecting line_number == 0 in parse_line_ref or in parse_line_content before stripping.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Read existing file content in spawn_approval_handler before generating diff preview, so Write tool shows correct diff when overwriting files. Also remove oneOf schema constraint from Edit tool that Anthropic API does not support at top level - runtime validation handles mode detection.
…rameter Wave 1 of code-editing-agent adaptations: - Add Levenshtein fuzzy matching module (fuzzy.rs) with similarity search - Add file read guards module (guards.rs) with binary detection and ignore patterns - Add around_line parameter to Read tool for centered line reads - 38 new unit tests across all three modules
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/uira-orchestration/src/tools/builtins/guards.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/builtins/guards.rs:73">
P2: `is_ignored_path` only matches `/` separators, so ignored directories won’t be detected on Windows paths. Normalize path separators (or inspect `Path` components) before applying the substring checks.</violation>
</file>
<file name="crates/uira-orchestration/src/tools/builtins/fuzzy.rs">
<violation number="1" location="crates/uira-orchestration/src/tools/builtins/fuzzy.rs:2">
P3: This module is currently unused in the crate and suppresses the dead-code lint, which leaves a block of unreferenced functionality in production. Consider removing it until it’s used or integrating it where needed instead of silencing the warning.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| /// | ||
| /// Returns `true` if the path should be ignored, `false` otherwise. | ||
| pub fn is_ignored_path(path: &Path) -> bool { | ||
| let path_str = path.to_string_lossy(); |
There was a problem hiding this comment.
P2: is_ignored_path only matches / separators, so ignored directories won’t be detected on Windows paths. Normalize path separators (or inspect Path components) before applying the substring checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/uira-orchestration/src/tools/builtins/guards.rs, line 73:
<comment>`is_ignored_path` only matches `/` separators, so ignored directories won’t be detected on Windows paths. Normalize path separators (or inspect `Path` components) before applying the substring checks.</comment>
<file context>
@@ -0,0 +1,341 @@
+///
+/// Returns `true` if the path should be ignored, `false` otherwise.
+pub fn is_ignored_path(path: &Path) -> bool {
+ let path_str = path.to_string_lossy();
+
+ // Check for node_modules
</file context>
| @@ -0,0 +1,391 @@ | |||
| //! Levenshtein fuzzy matching for finding similar strings in content. | |||
| #![allow(dead_code)] | |||
There was a problem hiding this comment.
P3: This module is currently unused in the crate and suppresses the dead-code lint, which leaves a block of unreferenced functionality in production. Consider removing it until it’s used or integrating it where needed instead of silencing the warning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/uira-orchestration/src/tools/builtins/fuzzy.rs, line 2:
<comment>This module is currently unused in the crate and suppresses the dead-code lint, which leaves a block of unreferenced functionality in production. Consider removing it until it’s used or integrating it where needed instead of silencing the warning.</comment>
<file context>
@@ -0,0 +1,391 @@
+//! Levenshtein fuzzy matching for finding similar strings in content.
+#![allow(dead_code)]
+//!
+//! This module provides utilities for finding similar strings when exact matches fail,
</file context>
…tion Wave 2 of code-editing-agent adaptations: - Enhance Edit tool error messages with fuzzy matching suggestions (Task 4) - Add standardized tool output formatting trait at tools/output.rs (Task 5) - Add todo continuation injection in agent loop (Task 6) - 15 new tests across all three features
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/uira-agent/src/continuation.rs">
<violation number="1" location="crates/uira-agent/src/continuation.rs:30">
P2: `is_completion_signal` treats negated phrases like "not finished" or "not all tasks" as completion signals because it only checks for raw substrings. This can incorrectly inject a continuation when the response explicitly says work is not complete. Consider guarding against negated forms when matching phrases.</violation>
</file>
<file name="crates/uira-agent/src/agent.rs">
<violation number="1" location="crates/uira-agent/src/agent.rs:960">
P2: continuation_count is meant to be per-run but is never reset. After hitting max_continuations once, future runs in the same agent instance will never inject a todo continuation, even when incomplete todos remain. Reset continuation_count at the start of each run (e.g., when run_prompt_owned/run_message begins).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "all tasks", | ||
| "work is complete", | ||
| ]; | ||
| completion_phrases.iter().any(|phrase| lower.contains(phrase)) |
There was a problem hiding this comment.
P2: is_completion_signal treats negated phrases like "not finished" or "not all tasks" as completion signals because it only checks for raw substrings. This can incorrectly inject a continuation when the response explicitly says work is not complete. Consider guarding against negated forms when matching phrases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/uira-agent/src/continuation.rs, line 30:
<comment>`is_completion_signal` treats negated phrases like "not finished" or "not all tasks" as completion signals because it only checks for raw substrings. This can incorrectly inject a continuation when the response explicitly says work is not complete. Consider guarding against negated forms when matching phrases.</comment>
<file context>
@@ -0,0 +1,106 @@
+ "all tasks",
+ "work is complete",
+ ];
+ completion_phrases.iter().any(|phrase| lower.contains(phrase))
+}
+
</file context>
|
|
||
| // Check if we should inject a continuation message | ||
| if self.continuation_enabled | ||
| && self.continuation_count < self.max_continuations |
There was a problem hiding this comment.
P2: continuation_count is meant to be per-run but is never reset. After hitting max_continuations once, future runs in the same agent instance will never inject a todo continuation, even when incomplete todos remain. Reset continuation_count at the start of each run (e.g., when run_prompt_owned/run_message begins).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/uira-agent/src/agent.rs, line 960:
<comment>continuation_count is meant to be per-run but is never reset. After hitting max_continuations once, future runs in the same agent instance will never inject a todo continuation, even when incomplete todos remain. Reset continuation_count at the start of each run (e.g., when run_prompt_owned/run_message begins).</comment>
<file context>
@@ -938,22 +947,63 @@ impl Agent {
+
+ // Check if we should inject a continuation message
+ if self.continuation_enabled
+ && self.continuation_count < self.max_continuations
+ && crate::continuation::is_completion_signal(&output)
+ && self
</file context>
…tests Wave 3 of code-editing-agent adaptations: - Apply standardized output sections to Read and Edit tools (Task 7) - Add 7 fuzzy Edit integration tests including Unicode and performance (Task 8) - Read output now has METADATA and CONTENT sections - Edit output now has DIFF section
- guards.rs: add should_ignore_file(path, cwd) matching plan Task 3 signature - edit.rs: extract format_error_with_suggestions() helper per plan Task 4 - edit.rs: fix long string test to >1000 chars, add RTL Unicode test per Task 8 - continuation.rs: add check_todo_continuation() per plan Task 6 - continuation.rs: populate system_injection in ContinuationMessage - All 72+ new tests pass, clippy clean
junhoyeo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Readoutput to includefile_hash,range, andLINE#ID | contenttagsEditto support hashline edit ops (replace/append/prepend) with requiredexpected_file_hash, while preserving legacyold_string/new_stringbehaviorValidation
Summary by cubic
Adds a hashline-native Read/Edit flow anchored to LINE#ID with stale-safe hashing and standardized output sections. Wires todo continuation into the agent stop path and hardens guards, comment hooks, and recovery to make edits safer and more reliable.
New Features
Migration
Written for commit e9e07e0. Summary will update on new commits.