fix(hashline): corrected no-op detection to check array length before comparing lines
- Fixed line normalization to trim trailing whitespace and strip carriage returns instead of removing all whitespace. - Fixed no-op detection to check array length equality before comparing lines, preventing false classification of multi-line expansions. - Added clarification to hashline tool documentation on `replace` operation semantics and `lines` boundary constraints. - Added test case validating single-line to multi-line expansion behavior and firstChangedLine calculation.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added subcommands to `/copy` command: `code` (copy last code block), `all` (copy all code blocks), `cmd` (copy last bash/python command), and `last` (copy full message)
|
||||
@@ -15,6 +16,7 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Updated hashline tool documentation with explicit guidance on `replace` operation semantics, clarifying that `lines` must not extend past `end` to avoid unintended line duplication
|
||||
- Improved diagnostic message formatting to group errors by file path with indented details for better readability
|
||||
- Modified eager todo prelude to use hidden custom message type instead of visible developer message, preventing duplicate prompt text in session history
|
||||
- Updated eager todo prompt to remove dynamic user request injection, simplifying the template and preventing request repetition in displayed messages
|
||||
@@ -28,6 +30,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed hashline line normalization to trim trailing whitespace and strip carriage returns instead of removing all whitespace, preserving intentional spacing in code
|
||||
- Fixed noop detection in hashline replace operations to check array length equality before comparing lines, preventing false noop classification when single-line replacements expand to multiple lines
|
||||
- Fixed path resolution to accept bare directory names without trailing slashes in comma/space-separated path lists (e.g., `apps packages phases`)
|
||||
- Per-role `modelRoles` thinking selectors now propagate through commit/title helper model selection, legacy commit analysis, and agentic commit sessions while preserving default thinking inheritance when no role override is configured
|
||||
|
||||
|
||||
@@ -33,16 +33,13 @@ const RE_SIGNIFICANT = /[\p{L}\p{N}]/u;
|
||||
/**
|
||||
* Compute a short hexadecimal hash of a single line.
|
||||
*
|
||||
* Uses xxHash32 on a whitespace-normalized line, truncated to 2 chars from
|
||||
* Uses xxHash32 on a trailing-whitespace-trimmed, CR-stripped line, truncated to 2 chars from
|
||||
* {@link NIBBLE_STR}. For lines containing no alphanumeric characters (only
|
||||
* punctuation/symbols/whitespace), the line number is mixed in to reduce hash collisions.
|
||||
* The line input should not include a trailing newline.
|
||||
*/
|
||||
export function computeLineHash(idx: number, line: string): string {
|
||||
if (line.endsWith("\r")) {
|
||||
line = line.slice(0, -1);
|
||||
}
|
||||
line = line.replace(/\s+/g, "");
|
||||
line = line.replace(/\r/g, "").trimEnd();
|
||||
|
||||
let seed = 0;
|
||||
if (!RE_SIGNIFICANT.test(line)) {
|
||||
@@ -626,7 +623,7 @@ export function applyHashlineEdits(
|
||||
if (!edit.end) {
|
||||
const origLines = originalFileLines.slice(edit.pos.line - 1, edit.pos.line);
|
||||
const newLines = edit.lines;
|
||||
if (origLines.every((line, i) => line === newLines[i])) {
|
||||
if (origLines.length === newLines.length && origLines.every((line, i) => line === newLines[i])) {
|
||||
noopEdits.push({
|
||||
editIndex: idx,
|
||||
loc: `${edit.pos.line}#${edit.pos.hash}`,
|
||||
|
||||
@@ -30,6 +30,8 @@ Before choosing the payload, answer these questions in order:
|
||||
- `[""]` — blank line
|
||||
- `null` or `[]` — delete if replace, no-op if append or prepend
|
||||
|
||||
`replace` substitutes **exactly** the `pos..end` range (inclusive). Everything before `pos` and after `end` is preserved untouched. If `lines` contains content that already exists after `end`, those lines **will be duplicated** in the output. Keep `lines` to exactly what belongs inside the consumed range.
|
||||
|
||||
Ops are applied bottom-up. Tags **MUST** be referenced from the most recent `read` output.
|
||||
</operations>
|
||||
|
||||
@@ -117,6 +119,21 @@ Blank out a line without removing it:
|
||||
```
|
||||
</example>
|
||||
|
||||
<example name="delete a single line from a block">
|
||||
Remove just the TODO comment (line 10) from `beta()` without touching anything else:
|
||||
```
|
||||
{
|
||||
path: "util.ts",
|
||||
edits: [{
|
||||
op: "replace",
|
||||
pos: {{hlineref 10 "\t// TODO: remove after migration"}},
|
||||
lines: null
|
||||
}]
|
||||
}
|
||||
```
|
||||
Do **not** widen the range and re-emit surrounding lines when a single-line delete suffices.
|
||||
</example>
|
||||
|
||||
<example name="rewrite a block body — shape (a)">
|
||||
Replace the catch body with smarter error handling. Shape (a): `pos` is the first body line, `end` is the last body line. The catch header (line 14) and its closer (line 17) are outside the range and stay untouched.
|
||||
```
|
||||
@@ -271,5 +288,6 @@ Good — prepend before the next declaration so the new sibling is anchored on a
|
||||
- When changing existing code near a block tail or closing delimiter, default to `replace` over the owned span instead of inserting around the boundary.
|
||||
- When adding a sibling declaration, default to `prepend` on the next sibling declaration instead of `append` on the previous block's closing brace.
|
||||
- **Block boundaries travel together.** For a block `{ header / body / closer }`, there are exactly two valid replace shapes: (a) replace only the body — `pos`=first body line, `end`=last body line, leave the header and closer untouched; or (b) replace the whole block — `pos`=header, `end`=closer, re-emit all three in `lines`. Never split them: do not set `end` to the closer while omitting it from `lines` (deletes it), and do not emit the closer in `lines` without including it in `end` (duplicates it). This applies to every block terminator: `}`, `continue`, `break`, `return`, `throw`.
|
||||
- **`lines` must not extend past `end`.** `lines` replaces exactly `pos..end`. Content after `end` survives. If you include lines in `lines` that exist after `end`, they will appear twice. Either extend `end` to cover all lines you are re-emitting, or remove the extra lines from `lines`.
|
||||
- `lines` entries **MUST** be literal file content with indentation copied exactly from the `read` output. If the file uses tabs, use a real tab character.
|
||||
</critical>
|
||||
@@ -699,6 +699,15 @@ describe("applyHashlineEdits — multiple edits", () => {
|
||||
expect(result.lines).toBe("one\nTWO_THREE\nfour\nfive\nSIX");
|
||||
});
|
||||
|
||||
it("single-line replace expanding to multiple lines is not a noop", () => {
|
||||
const content = "aaa\n\nccc";
|
||||
const blankHash = computeLineHash(2, "");
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: { line: 2, hash: blankHash }, lines: ["", "inserted", ""] }];
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("aaa\n\ninserted\n\nccc");
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("empty edits array is a no-op", () => {
|
||||
const content = "aaa\nbbb";
|
||||
const result = applyHashlineEdits(content, []);
|
||||
|
||||
Reference in New Issue
Block a user