Skip to content

M14: validated source editing across Wright frontends (autonomous checkpoint) - #133

Merged
Teakowa merged 5 commits into
mainfrom
feat/m14-validated-source-editing
Aug 16, 2026
Merged

Teakowa merged 5 commits into
mainfrom
feat/m14-validated-source-editing

Conversation

@Teakowa

@Teakowa Teakowa commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Autonomous Goal

Advance Wright M14 validated source editing as far as safely possible through the currently approved M14 issues (#128 → #129 → #130/#131 → #132), maintaining this Draft PR as the persistent checkpoint. No new roadmap scope, refactoring categories, or OPY/OSTW compatibility broadening without a concrete blocker.

Base

main @ 25f21e6 (feat(driver): integrate Workshop→OPY/OSTW reconstruction behind one shared convert contract (#126))

Status

REVIEW_REQUIRED (PM re-verification of 264fa43)

Completed Checkpoints

264fa43 — #128 acceptance blockers fixed (PM review round 2)

Evidence per blocker:

  • Blocker 1 (range drift): apply_transaction applies every range against one original source snapshot — per source, edits apply in descending position order, so earlier replacements' length/newline changes never shift later ranges; EditTransaction::apply is the mechanical public application. Regressions: same-line length-changing edits (same_line_edits_apply_against_the_original_snapshot_when_length_changes), multiline deletion followed by a later edit (later_edit_after_a_multiline_replacement_keeps_original_coordinates), and longer/shorter same-line semantic rename (semantic_rename_same_line_occurrences_with_longer_and_shorter_names).
  • Blocker 2 (silent clamping): apply_edit validates every column strictly (1-based, in 1..=char_count+1); column 0 or beyond-line columns refuse with edit-invalid-range, never clamp (invalid_columns_are_rejected_not_clamped).
  • Blocker 2 (zero-width): order-dependent zero-width combinations at one position refuse at construction with edit-zero-width-conflict (same-position insertions and insert-at-another-edit's-start), with a documented deterministic rule (same_position_zero_width_edits_are_refused_as_conflicts).
  • Blocker 3 (preview on failure): validate_transaction returns ok=false and no validated preview whenever any check fails, including compiled/semantic-invalid edited projects; the broken-include regression now asserts preview.is_none() (was tautological) and compile_invalid_transaction_returns_no_validated_preview covers both transaction and rename paths.
  • Also fixed a parallel-test race in temp fixture dir names (two OSTW tests shared one directory).
  • Validation: cargo test --workspace --all-targets --all-features green (55 suites; driver edit suite 20/20 across 5 consecutive runs), clippy -D warnings clean, cargo fmt --check clean; PR CI: all 22 checks pass on 264fa43.

#132 acceptance verification — independent verification of #128-#131 (no code changes)

Evidence (all run at commit da89ab6, no issue-state reliance):

Post-M14 reassessment (draft for PM review — not ratified by the autonomous run)

M14 established one validated source-mutation architecture: frontend-neutral transactions (#128), shared identity-based rename (#129), transport-neutral tool mutation (#130), and a pure LSP adapter (#131). Recommended boundary going forward: do NOT auto-expand into extract-method/inline-function/formatter/general AST mutation or a plugin ABI without concrete agent/editor consumer evidence and a PM-led milestone; #96 remains deferred; OPY/OSTW compatibility breadth unchanged (no compatibility defects blocked any M14 acceptance criterion).

#132 acceptance verification — independent verification of #128-#131 (no code changes)

Evidence (all run at commit da89ab6, no issue-state reliance):

Post-M14 reassessment (draft for PM review — not ratified by the autonomous run)

M14 established one validated source-mutation architecture: frontend-neutral transactions (#128), shared identity-based rename (#129), transport-neutral tool mutation (#130), and a pure LSP adapter (#131). Recommended boundary going forward: do NOT auto-expand into extract-method/inline-function/formatter/general AST mutation or a plugin ABI without concrete agent/editor consumer evidence and a PM-led milestone; #96 remains deferred; OPY/OSTW compatibility breadth unchanged (no compatibility defects blocked any M14 acceptance criterion).

da89ab6 — #131 language services: LSP rename converges on the shared contract

Evidence:

  • Rename edits are now the shared [M14 refactoring] Unify semantic rename across OPY and OSTW on validated edit transactions #129 transaction in editor conventions: exact-occurrence 0-based UTF-16 ranges (one edit per occurrence, never whole-document replacement); the language-service result carries the validated per-source previews (previews).
  • The LSP adapter is a pure mapping: groups the exact edits per document into documentChanges/TextDocumentEdit with per-document versions (null for filesystem-backed sources); no protocol-layer symbol/collision/stale logic; unsupported targets surface the shared refusal as an explicit LSP error.
  • Protocol regressions: multi-document OPY rename, OSTW rename through the same adapter (new), UTF-16/non-BMP ranges (new), version preconditions, string/comment isolation (edit lines asserted), unresolvable refusals.
  • M10 rename tests updated to assert exact occurrence edits and preview equality; cargo test --workspace green (65 suites); clippy/fmt clean.

e3f4901 — #130 agent tooling: validated mutation through the shared tool API

Evidence:

  • ToolRequest::validateEditTransaction and ToolRequest::semanticRename over the session project with all-or-nothing semantics, structured refusal diagnostics, and per-source previews carrying identity/version preconditions.
  • Capabilities advertise the mutation operations; the JSON-RPC adapter was fixed to forward the full request shape (previously dropped all params except op) and is transport-equivalence tested for the mutation ops; in-process ToolService tests cover OPY + OSTW evidence cases.
  • cargo test --workspace green; clippy/fmt clean.

e8810ab — #128 foundation: source-edit transactions are project- and frontend-aware

Evidence:

  • wright-driver EditTransaction/validate_transaction replaces the OPY-hard-coded validate_edit/edit.opy path; edits carry per-source identity/version preconditions; stale/unknown/overlap/unsupported-kind refusals are deterministic and atomic (no partial preview).
  • OPY validation compiles through wright_opy::compile_with_overlay_outcome + the shared HIR→IR→lower→validate chain + session profile; OSTW validation compiles through wright_ostw::compile_with_semantics_overlay with the ds.toml project graph. Cross-file diagnostics keep real source paths (driver tests assert the span names the edited include/import file).
  • No filesystem write is required to preview/validate; the on-disk project is byte-unchanged after validation.
  • M10 rename validation in wright-language and the wright-consumer rename workflow now route through the shared contract; overlay_with_edits dead code removed.
  • cargo test --workspace green; cargo clippy --workspace --all-targets and cargo fmt --all clean.
  • New evidence: crates/wright-driver/tests/edit.rs (9 tests), wright-ostw/tests/parse.rs overlay test.

2583ca6 — #129 refactoring: semantic rename unified across OPY and OSTW on validated transactions

Evidence:

  • wright_driver::edit::semantic_rename is the shared refactoring: resolves the symbol at a 1-based position in a project source through the shared semantic index, refuses unresolved positions/collisions/occurrences without exact identifier spans, produces one exact-range multi-source transaction (declaration/definition/reference identifiers only), and validates through validate_transaction ([M14 foundation] Make source-edit transactions project- and frontend-aware #128) before reporting success. No LSP types, no mutable IR.
  • wright-language::rename delegates per affected root to the shared contract, unions the transactions (dedupe by source+range), keeps the stale guard and per-root kind-aware validation, and materializes the M10 RenameEdit shape for the LSP adapter. collision_problem/symbol_at_in_file/renamed_text/TargetSpan removed (duplicated semantics).
  • OSTW rename added on the declared semantic surface: identifier-exact name_span propagated through the OSTW CST/parser and [M13] Resolve and lower the accepted protect-ban OSTW semantic slice into HIR #118 semantic HIR for globals, player variables, functions/subroutines, and rule names; typed constants and other provenance-limited targets refuse explicitly (rename-unresolved-target).
  • Fixed the OSTW project registry identity: file 0 is ds.toml, not the main source — file matching walks the registry instead of assuming file 0.
  • New regressions: driver semantic_rename_* tests (same-spelled identity isolation, cross-file OPY, OSTW cross-file, collision/unresolved/empty-name refusals, breaking-rename refusal); language-service OSTW rename tests (cross-file edits, collision/unresolved refusals, open-overlay references). M10 cross-file fixtures moved to real file URIs per the path-based contract.
  • cargo test --workspace green (multiple consecutive runs; the one observed transient edit-test failure did not reproduce in 3 subsequent runs); clippy/fmt clean.

Current Work

None — all approved M14 implementation issues (#128-#131) are landed; the #128 acceptance blockers recorded in the prior PM review are fixed in 264fa43 with focused regressions and full PR CI green (22/22). Remaining steps require human review (PM re-verification of the blocker fixes, reassessment ratification, merge decision).

Validation

Latest full run (this commit): cargo test --workspace all suites ok; cargo clippy --workspace --all-targets and cargo fmt --all clean. Compatibility/oracle Python tests and pinned pnpm/Node oracle jobs not run locally (no fixture or corpus changes; CI will execute them on the PR).

Blockers

None for the autonomous run. Resolved note: cargo test does not run the compatibility/ Python oracle suite; that gate runs in CI.

Decisions Requiring Review

  • validate_transaction/semantic_rename compose the native frontends directly (mirroring the session.load chains) rather than routing through CompilerSession, because edited/main texts live in memory; the session remains the single owner of normal load workflows. M10 rename validation migrated from frontend-error-only to the full chain (strictly safer).
  • Path-based main input is required (stdin refuses edit-input-stdin); M9's stdin+temp-file path removed. Cross-file M10 fixtures updated to real file URIs (the shared contract is path-based).
  • SourceEdit gained a required source field; EditRange columns are 1-based character columns matching compiler spans.
  • OSTW rename surface: globals, player variables, subroutines/functions, and rule names; typed constants and provenance-limited targets refuse explicitly. This updates the [M13] Integrate the accepted OSTW semantic surface with Wright tooling services #120 "rename not offered for OSTW" documentation.
  • semantic_rename requires the caller's current-text snapshot (sources) for every file the rename may edit; disk fallback only for the main source.

Next Candidates

  • Post-M14: PM reassessment ratification (human) before any broader refactoring milestone.
  • PR merge decision (human).

Notes

Worktree: /private/tmp/wright-m14-autonomous (branch feat/m14-validated-source-editing). Git/repo evidence always overrides this checkpoint.

…re (#128)

Reconcile the M9 safe-edit contract with the current project/session architecture: one EditTransaction carries multi-file edits with exact source ranges and per-source identity/version preconditions, and validate_transaction runs the edited project through the correct native frontend (OPY via include overlays, OSTW via the ds.toml project graph with overlays) instead of forcing SourceKind::Opy through a synthetic edit.opy path. Refusals are atomic and structured (stale/unknown sources, overlaps, unsupported kinds, compiled errors) with cross-file span provenance, and validation never writes the filesystem. wright-language rename validation and the wright-consumer rename workflow now route through the shared contract; the OSTW frontend gains overlay-capable compile entry points.
…ansactions (#129)

Make semantic rename a shared refactoring over the project-aware transaction contract: wright_driver::edit::semantic_rename resolves the symbol at a position through the shared semantic index of the project compiled by its original native frontend, produces one exact-range multi-source transaction (declaration/definition/reference occurrences only), and validates it through validate_transaction (#128) before success. The wright-language rename now delegates per affected root to the shared contract and unions the transactions, preserving M10 project-wide behavior; OSTW rename is added on the declared semantic surface (globals, player variables, subroutines/functions) by propagating identifier-exact name spans through the OSTW CST/parser and #118 semantic HIR, with provenance-limited targets refusing explicitly. Cross-file M10 fixtures now use real file URIs per the path-based contract.
… the shared tool API (#130)

Add ToolRequest::validateEditTransaction and ToolRequest::semanticRename over the shared #128/#129 contracts: agents can request validated source-edit previews and semantic rename through Wright-owned transport-neutral operations with all-or-nothing refusal semantics, structured diagnostics, and per-source previews carrying identity/version preconditions. Capabilities advertise the mutation operations; the stdio/JSON-RPC adapters forward the full request shape (the JSON-RPC adapter previously dropped all parameters except op) and are transport-equivalence tested for OPY and OSTW evidence cases. Docs record that Wright proposes/validates edits while filesystem application stays an explicit consumer responsibility.
…tract (#131)

Rename edits are now the shared #129 transaction in editor conventions: exact-occurrence 0-based UTF-16 ranges (one edit per semantic occurrence, never a whole-document replacement), and the language-service result carries the validated per-source previews. The LSP adapter is a pure mapping — grouping the exact edits per document into documentChanges/TextDocumentEdit with per-document versions and the unversioned null form for filesystem-backed sources — with no protocol-layer symbol resolution, collision, or stale checks. Protocol regressions cover multi-document OPY rename, supported OSTW rename through the same adapter, UTF-16/non-BMP ranges, version preconditions, and unsupported-target refusals; M10 rename tests now assert exact occurrence edits and preview equality.

Teakowa commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

PM verification: CHANGES_REQUIRED

The M14 branch is substantial and the architecture is mostly aligned, but the current da89ab6 cannot be accepted yet. Independent code inspection found source-edit correctness defects in the #128 transaction foundation that current green CI does not cover.

Acceptance blocker 1 — later edit ranges drift after earlier replacements

EditTransaction::new orders edits for one source by ascending source position. apply_transaction then applies those original-source ranges sequentially to the already-modified preview text.

If an earlier edit changes text length, a later range on the same line no longer addresses the original semantic occurrence. If an earlier edit inserts/removes newlines, later line/column coordinates can drift more broadly. This can incorrectly reject a valid semantic rename or, worse, mutate the wrong range if the resulting source still compiles.

A common repro shape is multiple references on one line, e.g. score = score + score, renamed to a different-length identifier.

The transaction contract must apply all ranges against the same original source snapshot, e.g. by applying non-overlapping edits in descending source order per file or by a single-pass reconstruction.

Acceptance blocker 2 — invalid columns are silently clamped

apply_edit validates line numbers/order but char_col clamps invalid columns to the start/end of a line. Under the declared 1-based exact-range contract, col = 0 or a column beyond char_count + 1 must be rejected, not silently redirected to a different location.

This is especially important for agent-facing edits because a malformed range must never become a different valid edit.

Also explicitly classify same-position zero-width insertions (and other order-dependent zero-width combinations) as conflicts or define a documented deterministic semantic. The current overlap check allows two zero-width edits at the same position.

Acceptance blocker 3 — compile-invalid transactions still return a preview

After compile_project returns errors, validate_transaction extends diagnostics but still returns preview: Some(previews). That contradicts the module/issue contract that compiled/unsafe refusals are atomic and return no partial/validated preview.

The existing broken-include regression does not enforce this: its assertion accepts both None and Some and is effectively tautological.

For the current M14 contract, any failed validation should return ok = false and no validated preview. If an unvalidated candidate preview is desirable later, it should be a separately named/result-classified surface rather than reusing SourcePreview from a validated transaction.

Required regressions before acceptance

  • multiple non-overlapping edits on the same line where the first replacement changes length;
  • an earlier multiline insertion/deletion followed by a later edit in the same source;
  • semantic rename with multiple same-line occurrences and a longer/shorter new identifier;
  • column 0 and column past EOL refusal;
  • same-position zero-width edit conflict behavior;
  • compile/semantic-invalid transaction returns no validated preview.

The rest of the reviewed shape is sound: #129 uses exact semantic occurrences, #130 exposes the shared operations through ToolService/transports, and #131 maps the shared rename result into LSP rather than reimplementing semantic resolution. Keep those boundaries intact while fixing #128.

Do not merge or mark #132/M14 accepted until these regressions are fixed and the full PR CI is green again.

…tion (#133)

Apply every transaction range against one original source snapshot: per source, edits apply in descending position order so an earlier replacement's length or newline changes can never shift a later range (EditTransaction::apply is the mechanical public application). Columns are strict 1-based character columns: 0 or beyond-line columns refuse with edit-invalid-range instead of clamping, and order-dependent zero-width combinations at one position refuse with edit-zero-width-conflict. Validation is atomic end to end: any failed check, including compiled/semantic-invalid edited projects, returns ok=false and no validated preview. Focused regressions cover same-line length-changing edits, multiline coordinate stability, longer/shorter same-line semantic rename, invalid columns, zero-width conflicts, and compile-invalid transactions; the broken-include preview assertion is no longer tautological. Temp fixture dirs are unique per test call to remove a parallel-test race.

Teakowa commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

PM re-verification — #128 blockers resolved at 264fa43

Rechecked the actual implementation and focused regressions, not only the PR status.

The three prior #128 correctness blockers are resolved:

  1. Original-snapshot ranges: same-source edits are grouped and applied in descending source position, so every stored range continues to address the original snapshot. The focused tests cover same-line length changes, multiline coordinate shifts, and longer/shorter semantic rename with multiple occurrences.
  2. Strict range validity: 1-based columns are validated against the actual line length; column 0 and past-EOL columns now refuse with edit-invalid-range. Same-position/order-dependent zero-width combinations refuse deterministically with edit-zero-width-conflict.
  3. Atomic validation: any project/frontend validation error now returns ok=false and preview=None; the previous tautological broken-include assertion was replaced with a real no-preview assertion.

PR head 264fa43 also has 22/22 GitHub checks successful, including Rust/MSRV, compatibility/reference/differential gates and cross-platform distribution.

I found no remaining architecture blocker in the #128 repair. The shared #129/#130/#131 boundaries remain intact.

One acceptance sequencing correction remains: the #132 independent acceptance recorded in the PR body was originally run at da89ab6. Because the transaction core changed afterward, final #132 acceptance must explicitly verify the current head (264fa43 or its eventual successor), rather than inheriting the older acceptance result. This is an acceptance/evidence requirement, not a request for further implementation scope.

@Teakowa
Teakowa marked this pull request as ready for review August 16, 2026 05:45
@Teakowa
Teakowa merged commit fb0d57b into main Aug 16, 2026
22 checks passed
@Teakowa

Teakowa commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

M14 #132 acceptance — independent verification at PR #133 head 264fa43

Verdict: ACCEPTED. All gating steps green at the current PR head; no correctness/architecture blocker found. This is an independent re-verification from the repo state at execution time — no issue-comment or implementation-agent claim was relied upon.

Verified at execution time (2026-08-16):

#128 — source-edit transactions (re-checked, including post-acceptance blocker fixes)

cargo test -p wright-driver --test edit run twice: 20/20 pass both runs (captured to {SCRATCH}/driver-edit.log). The six blocker regressions plus the broken-include preview regression exist verbatim in crates/wright-driver/tests/edit.rs at the head and pass:

  • same_line_edits_apply_against_the_original_snapshot_when_length_changes
  • later_edit_after_a_multiline_replacement_keeps_original_coordinates
  • semantic_rename_same_line_occurrences_with_longer_and_shorter_names
  • invalid_columns_are_rejected_not_clamped
  • same_position_zero_width_edits_are_refused_as_conflicts
  • compile_invalid_transaction_returns_no_validated_preview
  • opy_broken_include_edit_refuses_with_the_include_path_in_the_span (preview None on failure)

By-inspection of the shipped contract (crates/wright-driver/src/edit.rs): apply_transaction groups per source and applies edits in descending position order against one original snapshot (no coordinate drift); apply_edit validates every column strictly 1-based in 1..=char_count+1 and refuses with edit-invalid-range (never clamps); EditTransaction::new refuses order-dependent zero-width combinations at one position with edit-zero-width-conflict and overlaps with edit-overlap; validate_transaction returns ok=false with no validated preview on any failure (stale/unknown sources, malformed ranges, or compiled/semantic-invalid edited projects) and validates through the correct native frontend for the original source kind (OPY with include overlays, OSTW with the ds.toml project graph) — no synthetic edit.opy, no filesystem write.

#129 — semantic rename (shared, identity-based)

  • Driver: the semantic_rename_* cases pass in the edit suite above (identity-only edits, cross-file OPY, cross-file OSTW, collision/unresolved/breaking refusals, same-line longer/shorter names).
  • Language service: cargo test -p wright-language → 59 tests pass (document 8, service 9, cross_file 6, overlay 35, perf 1), including rename_edit_ranges_use_utf16_offsets_after_non_bmp_text and the overlay rename cases (captured to {SCRATCH}/rename.log).
  • By-inspection (crates/wright-language/src/service.rs): rename delegates per affected open root to wright_driver::edit::semantic_rename (shared semantic index resolves the exact identity; only that identity's occurrence spans are edited as exact-range edits), unions + dedupes the transactions, and validates through wright_driver::edit::validate_transaction against every affected root before success. No plain-text whole-document replacement exists in the rename path (no source replace(/match_indices); refusals — unresolved, collision, stale source, failed edited-project validation — are explicit ok=false results.

#130 — ToolService mutation (in-process and transport agree)

  • cargo test -p wright-driver --test service → 14/14 pass (OPY + OSTW validate-edit/semantic-rename through ToolService); cargo test -p wright-cli --test serve → 7/7 pass (stdio + JSON-RPC transport, incl. transports_are_equivalent_for_mutation_operations) (captured to {SCRATCH}/serve-mutation.log).
  • Launch check through the real wright-serve binary ({SCRATCH}/serve-mutation-launch.log): validateEditTransaction run twice over stdio — both responses are the full envelope (ok:true, exit:0, per-source preview with new_text + source_identity), byte-identical across the two runs; semanticRename over JSON-RPC returns the full envelope (ok:true, exact-range transaction edits, validated preview); a target-collision rename (score→hasStarted) refused identically twice with structured rename-collision, ok:false, transaction:null — all-or-nothing, never partial.
  • By-inspection: ToolRequest::ValidateEdit/SemanticRename are structured, transport-neutral owned types handled by calling the shared validate_transaction/semantic_rename; the stdio/JSON-RPC adapters are thin mappings with no semantic logic; capability advertisement includes both operations.

#131 — LSP adapter (pure mapping over the shared contract)

  • cargo test -p wright-lsp --test lsp run twice: 25/25 pass both runs (captured to {SCRATCH}/lsp.log), covering multi-document workspace edits, unversioned filesystem-backed form, per-document versions, unresolvable/stale refusals, strings/comments untouched, OSTW through the same adapter, and UTF-16/non-BMP positions.
  • By-inspection (crates/wright-lsp/src/main.rs): the textDocument/rename handler calls the shared service.rename(...) and maps the result to documentChanges/TextDocumentEdit with per-document versions (source_version = document version, None for filesystem-backed), one TextEdit per semantic occurrence, no changes-only fallback; shared refusals surface as explicit LSP errors (-32602). No protocol-layer symbol/collision/stale logic exists.

Full local gate set (all run at the head)

cargo fmt --all -- --check clean; cargo clippy --workspace --all-targets --all-features -- -D warnings clean; cargo test --workspace --all-targets --all-features green (55 suites, 0 failures); python3 -m unittest discover -s compatibility/tests 18 tests OK (1 skipped: the manifest probe needing the pinned OverPy oracle); python3 scripts/v1-gates.py pass; python3 scripts/run-scenarios.py 7/7 pass; cargo run -p wright-bench no regressions. All reports written under target/ (v1-gates, scenarios, bench, differential ×2, reconstruct ×2, convert). Captured to {SCRATCH}/full-gates.log.
Network/platform-bound oracle jobs were not locally runnable (compatibility/oracle/node_modules absent — pinned pnpm deps; OSTW v3.4.0 reference absent — needs network acquire): the local limit is recorded in full-gates.log, and both are green on the head's CI ("OverPy compatibility oracle", "OSTW reference baseline").

Architecture invariants (grep over the head tree; {SCRATCH}/inspection.log)

No production change

The acceptance run modified no production behavior and created no commits. The working tree after the run shows only the pre-existing uncommitted change (crates/wright-driver/src/session.rs diag-helper visibility, matching the head's committed state) and the pre-existing untracked .opencode/.

Post-M14 reassessment

Ratification of the post-M14 roadmap reassessment remains a PM/human action; this acceptance references the draft already in this PR's body and makes no further scope claim.

Independent verification performed from repo state at 264fa43; evidence logs under {SCRATCH} (head-ci, driver-edit, rename, serve-mutation, serve-mutation-launch, lsp, full-gates, inspection).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant