diff --git a/docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md b/docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md new file mode 100644 index 0000000..9c5cce6 --- /dev/null +++ b/docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md @@ -0,0 +1,409 @@ +# LSP UTF-16 Text Coordinates Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Convert LSP UTF-16 positions into consistent UTF-8 byte offsets and Tree-sitter byte points. + +**Architecture:** `lsp.utils.text_coordinates` exposes one `ToBytePosition` operation that resolves both representations in a single scan, while `ToOffset` remains a wrapper for provider callers. Incremental parsing consumes the paired result so `TSInputEdit` byte offsets and points always refer to the same source location. + +**Tech Stack:** C++23 Modules, LSP 3.17 UTF-16 positions, UTF-8, Tree-sitter, CMake, CTest + +--- + +## Plan Meta + +- **Plan Group:** text-coordinates-utf16 +- **Parent Plan:** none +- **Verification Scope:** text-coordinate regression tests, provider test suite, server build, LSP transport smoke test +- **Verification Gate:** `test_provider` passes, `tsl-server` builds, changed C++ files pass clang-format, and the LSP JSON transport completes with response validation disabled +- **Execution Constraints:** `karpathy-guidelines`, `.agents`, `AGENT_RULES.md`, test-driven development + +## File Map + +- Create `lsp-server/test/test_provider/text_coordinates_test.cppm`: focused UTF-16-to-byte coordinate regression tests. +- Modify `lsp-server/test/test_provider/CMakeLists.txt`: compile the new test module in `test_provider`. +- Modify `lsp-server/test/test_provider/test_main.cppm`: register the text-coordinate tests. +- Modify `lsp-server/src/utils/text_coordinates.cppm`: add paired byte-position resolution and correct UTF-16 decoding. +- Modify `lsp-server/src/manager/parser.cppm`: build `TSInputEdit` from paired coordinate results. + +### Task 1: Drive UTF-16 coordinate conversion with regression tests + +**Files:** + +- Create: `lsp-server/test/test_provider/text_coordinates_test.cppm` +- Modify: `lsp-server/test/test_provider/CMakeLists.txt` +- Modify: `lsp-server/test/test_provider/test_main.cppm` + +- [ ] **Step 1: Add the focused test module** + +Create `lsp-server/test/test_provider/text_coordinates_test.cppm`: + +```cpp +module; + +export module lsp.test.provider.text_coordinates; + +import std; + +import lsp.protocol; +import lsp.test.framework; +import lsp.utils.text_coordinates; + +export namespace lsp::test::provider +{ + class TextCoordinatesTests + { + public: + static void Register(TestRunner& runner); + + private: + static TestResult TestAsciiAndBmpPositions(); + static TestResult TestSupplementaryPlanePositions(); + static TestResult TestMultilineAndClampedPositions(); + static TestResult TestMalformedUtf8IsBounded(); + static TestResult TestCalculateEndPointUsesByteColumns(); + }; +} + +namespace lsp::test::provider +{ + namespace + { + void ExpectPosition(const utils::text_coordinates::BytePosition& actual, + protocol::uinteger offset, + std::uint32_t row, + std::uint32_t column) + { + assertEqual(offset, actual.offset, "byte offset should match"); + assertEqual(row, actual.point.row, "Tree-sitter row should match"); + assertEqual(column, actual.point.column, "Tree-sitter byte column should match"); + } + } + + void TextCoordinatesTests::Register(TestRunner& runner) + { + runner.addTest("text coordinates convert ASCII and BMP positions", TestAsciiAndBmpPositions); + runner.addTest("text coordinates convert supplementary-plane positions", TestSupplementaryPlanePositions); + runner.addTest("text coordinates clamp multiline positions", TestMultilineAndClampedPositions); + runner.addTest("text coordinates bound malformed UTF-8", TestMalformedUtf8IsBounded); + runner.addTest("text coordinates calculate byte end points", TestCalculateEndPointUsesByteColumns); + } + + TestResult TextCoordinatesTests::TestAsciiAndBmpPositions() + { + const protocol::string content = "A中Z"; + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 0 }, content), 0U, 0U, 0U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 1 }, content), 1U, 0U, 1U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 4U, 0U, 4U); + assertEqual(4U, utils::text_coordinates::ToOffset({ 0, 2 }, content), + "ToOffset should use the paired conversion"); + return { "", true, "ok" }; + } + + TestResult TextCoordinatesTests::TestSupplementaryPlanePositions() + { + const protocol::string content = "A😀Z"; + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 1 }, content), 1U, 0U, 1U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 1U, 0U, 1U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 3 }, content), 5U, 0U, 5U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 4 }, content), 6U, 0U, 6U); + return { "", true, "ok" }; + } + + TestResult TextCoordinatesTests::TestMultilineAndClampedPositions() + { + const protocol::string content = "中x\n😀y"; + ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 0 }, content), 5U, 1U, 0U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 2 }, content), 9U, 1U, 4U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 100 }, content), 10U, 1U, 5U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 9, 0 }, content), 10U, 1U, 5U); + return { "", true, "ok" }; + } + + TestResult TextCoordinatesTests::TestMalformedUtf8IsBounded() + { + const protocol::string content{ 'A', static_cast(0xF0), static_cast(0x9F), 'Z' }; + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 2U, 0U, 2U); + ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 100 }, content), 4U, 0U, 4U); + return { "", true, "ok" }; + } + + TestResult TextCoordinatesTests::TestCalculateEndPointUsesByteColumns() + { + const auto end = utils::text_coordinates::CalculateEndPoint("中\n😀x", { 2U, 3U }); + assertEqual(3U, end.row, "newline should advance the Tree-sitter row"); + assertEqual(5U, end.column, "multibyte text should advance the byte column"); + return { "", true, "ok" }; + } +} +``` + +- [ ] **Step 2: Register the new test module in the provider target** + +Add `text_coordinates_test.cppm` beside the other test modules in both `SOURCES` and the CMake `FILE_SET` in `lsp-server/test/test_provider/CMakeLists.txt`: + +```cmake + text_coordinates_test.cppm +``` + +Import and register it in `lsp-server/test/test_provider/test_main.cppm`: + +```cpp +import lsp.test.provider.text_coordinates; +``` + +```cpp + std::cout << " - Text coordinate tests" << std::endl; + lsp::test::provider::TextCoordinatesTests::Register(runner); +``` + +- [ ] **Step 3: Build to verify the test is red** + +Run: + +```bash +cmake --build lsp-server/build/codex43-clean/Release --target test_provider -j2 +``` + +Expected: build fails because `BytePosition` and `ToBytePosition` do not exist. This proves the test requires the new paired conversion API. + +### Task 2: Implement one UTF-16-to-byte conversion path + +**Files:** + +- Modify: `lsp-server/src/utils/text_coordinates.cppm` +- Modify: `lsp-server/src/manager/parser.cppm` + +- [ ] **Step 1: Replace the independent point conversion with paired resolution** + +In the exported namespace of `lsp-server/src/utils/text_coordinates.cppm`, replace `ToPoint` with: + +```cpp + struct BytePosition + { + protocol::uinteger offset; + TSPoint point; + }; + + BytePosition ToBytePosition(const protocol::Position& position, const protocol::string& content); +``` + +In the implementation namespace, add a validated UTF-8 decoder and implement the paired conversion: + +```cpp + namespace + { + struct DecodedCharacter + { + std::size_t byte_count; + protocol::uinteger utf16_units; + }; + + DecodedCharacter DecodeCharacter(std::string_view content, std::size_t offset) + { + const auto lead = static_cast(content[offset]); + if ((lead & 0x80U) == 0) + return { 1, 1 }; + + std::size_t byte_count = 0; + std::uint32_t code_point = 0; + std::uint32_t minimum = 0; + if ((lead & 0xE0U) == 0xC0U) + { + byte_count = 2; + code_point = lead & 0x1FU; + minimum = 0x80U; + } + else if ((lead & 0xF0U) == 0xE0U) + { + byte_count = 3; + code_point = lead & 0x0FU; + minimum = 0x800U; + } + else if ((lead & 0xF8U) == 0xF0U) + { + byte_count = 4; + code_point = lead & 0x07U; + minimum = 0x10000U; + } + else + { + return { 1, 1 }; + } + + if (byte_count > content.size() - offset) + return { 1, 1 }; + + for (std::size_t index = 1; index < byte_count; ++index) + { + const auto continuation = static_cast(content[offset + index]); + if ((continuation & 0xC0U) != 0x80U) + return { 1, 1 }; + code_point = (code_point << 6U) | (continuation & 0x3FU); + } + + if (code_point < minimum || code_point > 0x10FFFFU || + (code_point >= 0xD800U && code_point <= 0xDFFFU)) + { + return { 1, 1 }; + } + + return { byte_count, code_point >= 0x10000U ? 2U : 1U }; + } + } + + BytePosition ToBytePosition(const protocol::Position& position, const protocol::string& content) + { + std::size_t offset = 0; + std::size_t line_start = 0; + std::uint32_t row = 0; + + while (offset < content.size() && row < position.line) + { + if (content[offset++] == '\n') + { + ++row; + line_start = offset; + } + } + + protocol::uinteger utf16_units = 0; + while (offset < content.size() && content[offset] != '\n' && + utf16_units < position.character) + { + const auto decoded = DecodeCharacter(content, offset); + const auto remaining = position.character - utf16_units; + if (decoded.utf16_units > remaining) + break; + + offset += decoded.byte_count; + utf16_units += decoded.utf16_units; + } + + return { + .offset = static_cast(offset), + .point = TSPoint{ + .row = row, + .column = static_cast(offset - line_start), + }, + }; + } + + protocol::uinteger ToOffset(const protocol::Position& position, const protocol::string& content) + { + return ToBytePosition(position, content).offset; + } +``` + +Keep `CalculateEndPoint` unchanged: Tree-sitter columns count UTF-8 bytes, so its byte iteration is correct. + +- [ ] **Step 2: Use paired positions for incremental Tree-sitter edits** + +Replace the separate offset and point conversions in `SyntaxTree::ApplyEdit` in `lsp-server/src/manager/parser.cppm`: + +```cpp + const auto start = utils::text_coordinates::ToBytePosition(change.range.start, content); + const auto old_end = utils::text_coordinates::ToBytePosition(change.range.end, content); + + TSInputEdit edit{}; + edit.start_byte = start.offset; + edit.old_end_byte = old_end.offset; + edit.new_end_byte = start.offset + change.text.length(); + edit.start_point = start.point; + edit.old_end_point = old_end.point; + edit.new_end_point = utils::text_coordinates::CalculateEndPoint(change.text, edit.start_point); +``` + +- [ ] **Step 3: Build and run the regression suite to verify green** + +Run: + +```bash +cmake --build lsp-server/build/codex43-clean/Release --target test_provider -j2 +ctest --test-dir lsp-server/build/codex43-clean/Release -R '^test_provider$' --output-on-failure +``` + +Expected: the build succeeds and CTest reports `100% tests passed, 0 tests failed out of 1`. + +- [ ] **Step 4: Format the changed C++ files and verify formatting** + +Run: + +```bash +clang-format -i \ + lsp-server/src/utils/text_coordinates.cppm \ + lsp-server/src/manager/parser.cppm \ + lsp-server/test/test_provider/text_coordinates_test.cppm \ + lsp-server/test/test_provider/test_main.cppm +clang-format --dry-run --Werror \ + lsp-server/src/utils/text_coordinates.cppm \ + lsp-server/src/manager/parser.cppm \ + lsp-server/test/test_provider/text_coordinates_test.cppm \ + lsp-server/test/test_provider/test_main.cppm +``` + +Expected: both commands exit successfully with no diagnostics. + +- [ ] **Step 5: Commit the implementation** + +```bash +git add \ + lsp-server/src/utils/text_coordinates.cppm \ + lsp-server/src/manager/parser.cppm \ + lsp-server/test/test_provider/CMakeLists.txt \ + lsp-server/test/test_provider/test_main.cppm \ + lsp-server/test/test_provider/text_coordinates_test.cppm +git commit -m "fix: convert LSP UTF-16 text coordinates" +``` + +### Task 3: Verify server integration + +**Files:** + +- Verify only; no additional source changes expected. + +- [ ] **Step 1: Build the production server and provider tests** + +Run: + +```bash +cmake --build lsp-server/build/codex43-clean/Release --target tsl-server test_provider -j2 +``` + +Expected: Ninja completes successfully. + +- [ ] **Step 2: Run provider and LSP transport verification** + +Run: + +```bash +ctest --test-dir lsp-server/build/codex43-clean/Release -R '^test_provider$' --output-on-failure +python lsp-server/test/run_lsp_json_tests.py \ + --server lsp-server/build/codex43-clean/Release/tsl-server \ + --no-validate +``` + +Expected: `test_provider` reports zero failures and the LSP script completes all request/response exchanges successfully. Response validation remains disabled because the repository has a known unrelated rename-response fixture failure. + +- [ ] **Step 3: Confirm the old point-only API has no callers** + +Run: + +```bash +rg -n 'text_coordinates::ToPoint|text::ToPoint|TSPoint ToPoint' lsp-server/src lsp-server/test +``` + +Expected: no matches. + +- [ ] **Step 4: Record completion through the repository main loop** + +After all verification gates pass, run: + +```bash +python docs/standards/playbook/scripts/main_loop.py finish \ + -plan docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md \ + -status done \ + -progress memory-bank/progress.md +``` + +Expected: the plan is recorded as `done`. Update the human-readable summary in `memory-bank/progress.md` to mention the UTF-16 coordinate fix and its passing verification, then commit that state update separately. diff --git a/memory-bank/progress.md b/memory-bank/progress.md index 52386b2..c496c08 100644 --- a/memory-bank/progress.md +++ b/memory-bank/progress.md @@ -50,7 +50,7 @@ phase: planning spec: docs/superpowers/specs/2026-07-12-text-coordinates-utf16-design.md -plan: docs/superpowers/plans/2026-07-11-args-parser-startup-errors.md +plan: docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md executor: executing-plans constraints: karpathy-guidelines,.agents,AGENT_RULES