Bind comments forward in element groups
With comments as leaf nodes, an inserted run under a commutative
parent can hold them, and grouping decides what a union merge keeps
adjacent. A standalone comment describes the item below it — the same
forward-binding shape as a rust attribute — so it joins the following
element's group, and a run *ending* in a comment refuses to group.
That turns the mergiraf probe `use crate::{foo, /* bar */}` vs
`/* baz */` into the insert-insert conflict it should be: the old
trivia representation deduped the two bare comma-inserts and silently
dropped both comments.
duplicate-insert now exempts extras: both branches repeating the same
marker comment in different places is legitimate repetition, not a
duplicate definition, so both copies land.
The design note covers the whole comments-as-nodes change, including
the lift in the previous commit.
Assisted-by: Claude Fable 5 via Claude Code
diff --git a/docs/notes/2026-07-15-comment-nodes.md b/docs/notes/2026-07-15-comment-nodes.md
new file mode 100644
index 0000000..aa49c67
--- /dev/null
+++ b/docs/notes/2026-07-15-comment-nodes.md
@@ -0,0 +1,67 @@
+# Comments as opaque leaf nodes
+
+*2026-07-15.*
+
+Comments were tree-sitter `extra` nodes, skipped at lift and carried
+only as inter-leaf trivia by span synthesis. That made comment *edits*
+invisible to the diff — the top remaining gap from the mergiraf
+comparison. The motivating failure: `use crate::{foo, /* bar */}` vs
+`/* baz */` looked like two identical comma-inserts, deduped, and the
+output dropped both comments where mergiraf conflicts.
+
+## Decision
+
+Lift extras as **opaque labeled leaves**: keep the extra node, do not
+lift its children, and let `label()` return its full source text.
+Everything downstream then treats a comment like any other labeled
+leaf — a comment edit is a relabel, concurrent distinct edits are a
+relabel-relabel conflict, and comment inserts ride the existing graft
+machinery.
+
+Opacity is what makes the representation uniform. Java and JSON
+comments are already leaves carrying their text, but rust comments
+have interior structure whose *body text lives in no leaf*: `// plain`
+parses as `line_comment` with a single anonymous `//` child, and
+`/* bar */` as `block_comment` holding only the `/*` and `*/` tokens.
+Lifted naively, two different comments would be structurally
+identical — same kinds, no labels — and the edit would stay invisible.
+Doc comments (`/// doc`) do carry an inner `doc_comment` leaf, but
+plain comments are the common case, and one representation for all
+comments beats two.
+
+## Consequences through the pipeline
+
+- Hashing and anchoring see comment text via the label; a unique
+ comment can anchor, duplicated ones (`// TODO`) are excluded by the
+ uniqueness rule as usual.
+- Phase 2's (kind, label) LCS never pairs comments whose text differs;
+ phase 3's Zhang–Shasha folds them as relabels within the comment
+ kind. The relabel-relabel and relabel-delete rules then cover
+ concurrent comment edits and edit-under-delete for free.
+- The arity rule must skip extra children: extras fill no grammar
+ slot and `node-types.json` does not admit them anywhere, so a
+ changed node holding a comment would otherwise false-conflict.
+- Element groups treat extras as forward-binding prefixes, like rust
+ attributes: a standalone comment describes the item below it, and a
+ union merge must not splice the other branch's code between a doc
+ comment and its function. A run *ending* in a comment refuses to
+ group — which is exactly what turns the `use_list` probe into the
+ insert-insert conflict mergiraf expects, instead of a broken union.
+- duplicate-insert exempts extras: both branches adding an identical
+ `// section` comment at different slots is legitimate, not a
+ duplicate definition.
+- A branch that only edits comment text is no longer "edit-free", so
+ the reformat preference does not fire for it; the edit lands through
+ the normal relabel path instead, which is strictly better.
+- The corpus's structural comparison (`Shape`) now covers comments, so
+ scenarios assert comment content, not just code shape.
+
+## Rejected alternative
+
+mergiraf-style comment *attachment* (folding a comment's text into the
+hash/identity of the node it documents). It makes comment edits look
+like edits of the attached code — coarser conflicts — and needs
+per-language attachment heuristics up front. Attachment may still be
+worth adding later for *matching* quality (a moved function should
+carry its doc comment), which is the move-detection milestone's
+territory.
diff --git a/src/merge.rs b/src/merge.rs
index 8e82525..3e69d21 100644
--- a/src/merge.rs
+++ b/src/merge.rs
@@ -1560,6 +1560,83 @@ mod tests {
)
}
+ #[test]
+ fn concurrent_distinct_comment_inserts_conflict() -> Result<(), Error> {
+ // The motivating probe from the mergiraf comparison: each
+ // branch appends its own comment after the comma. The runs end
+ // in a comment — a forward-binding prefix with its target
+ // outside the run — so they refuse to group and insert-insert
+ // fires, where the trivia representation deduped the two
+ // "identical" comma-inserts and dropped both comments.
+ assert_conflicts(
+ "rust",
+ "use crate::{foo};\n",
+ "use crate::{foo, /* bar */};\n",
+ "use crate::{foo, /* baz */};\n",
+ "insert-insert",
+ )?;
+ Ok(())
+ }
+
+ #[test]
+ fn doc_comments_travel_with_their_item_in_a_union() -> Result<(), Error> {
+ // Both branches insert a documented function at the same slot.
+ // Each comment binds forward into its function's element group,
+ // so the union cannot splice B's function between A's doc
+ // comment and A's function.
+ let o = parse("fn main() {}\n", "rust")?;
+ let a = parse("/// A.\nfn a() {}\nfn main() {}\n", "rust")?;
+ let b = parse("/// B.\nfn b() {}\nfn main() {}\n", "rust")?;
+ match merge_to_text(&o, &a, &b)? {
+ MergeResult::Merged(text) => {
+ let a_pos = text.find("/// A.");
+ let b_pos = text.find("/// B.");
+ assert!(a_pos.is_some() && b_pos.is_some(), "{text:?}");
+ let merged = parse(&text, "rust")?;
+ // Doc-comment spans include their trailing newline.
+ let order: Vec<_> = merged
+ .nodes()
+ .filter_map(|n| merged.label(n))
+ .filter(|l| l.starts_with("///") || *l == "a" || *l == "b" || *l == "main")
+ .collect();
+ assert_eq!(
+ order,
+ vec!["/// A.\n", "a", "/// B.\n", "b", "main"],
+ "{text:?}"
+ );
+ }
+ MergeResult::Conflicts(conflicts) => panic!("unexpected conflicts: {conflicts:?}"),
+ }
+ Ok(())
+ }
+
+ #[test]
+ fn identical_documented_insertions_dedupe_as_one_group() -> Result<(), Error> {
+ // The comment is part of the element group's hash sequence, so
+ // the group dedupes whole: one comment, one function.
+ assert_merges(
+ "rust",
+ "fn main() {}\n",
+ "/// Shared.\nfn util() {}\nfn main() {}\n",
+ "/// Shared.\nfn util() {}\nfn main() {}\n",
+ "/// Shared.\nfn util() {}\nfn main() {}\n",
+ )
+ }
+
+ #[test]
+ fn identical_comments_at_different_slots_merge() -> Result<(), Error> {
+ // duplicate-insert exempts extras: repeating the same marker
+ // comment in two places is not a duplicate definition, so both
+ // copies land where their branch put them.
+ assert_merges(
+ "rust",
+ "fn a() {}\nfn b() {}\n",
+ "// marker\nfn a() {}\nfn b() {}\n",
+ "fn a() {}\n// marker\nfn b() {}\n",
+ "// marker\nfn a() {}\n// marker\nfn b() {}\n",
+ )
+ }
+
#[test]
fn a_wrap_pulls_the_survivor_into_the_graft() -> Result<(), Error> {
// A wraps the array one level deeper; B leaves it alone.
diff --git a/src/rules.rs b/src/rules.rs
index 4132cf8..ec7bcd7 100644
--- a/src/rules.rs
+++ b/src/rules.rs
@@ -466,7 +466,9 @@ fn dedupes_together(
/// with a `name`/`key` field, so without this rule the union merge
/// silently lands the element twice. Separators are excluded (every
/// comma equals every other comma); named elements are
-/// name-collision's domain.
+/// name-collision's domain. Extras are excluded too: both branches
+/// writing the same `// section` comment in different places is
+/// legitimate repetition, not a duplicate definition.
fn duplicate_insert(ctx: &Ctx, edits_a: &[Edit], edits_b: &[Edit], found: &mut Vec<Conflict>) {
let elements = |tree: &Tree, matching: &Matching, script: &[Edit]| {
let mut map: HashMap<(NodeId, u64), Vec<(Slot, NodeId)>> = HashMap::new();
@@ -474,7 +476,7 @@ fn duplicate_insert(ctx: &Ctx, edits_a: &[Edit], edits_b: &[Edit], found: &mut V
let Edit::Insert(node) = edit else {
continue;
};
- if !tree.is_named(*node) || node_name(tree, *node).is_some() {
+ if !tree.is_named(*node) || tree.is_extra(*node) || node_name(tree, *node).is_some() {
continue;
}
let Some(slot) = insert_anchor(tree, matching, *node) else {
@@ -572,17 +574,22 @@ fn insert_slots(tree: &Tree, matching: &Matching, script: &[Edit]) -> HashMap<Sl
/// node together with the anonymous separators and forward-binding
/// prefixes preceding it (a JSON insertion is `, "k": v` — the comma
/// travels with its pair; a Rust `#[attr]` travels with the item it
-/// governs). `None` when trailing prefix nodes remain: a separator
-/// with no element cannot participate in a union merge, and a
-/// trailing attribute governs something *outside* the run, which a
-/// union would displace. Group-wise dedupe in the builder relies on
-/// the same decomposition.
+/// governs). Extras bind forward too: a standalone comment describes
+/// the item below it, and a union must not splice the other branch's
+/// code between a doc comment and its function. `None` when trailing
+/// prefix nodes remain: a separator with no element cannot participate
+/// in a union merge, and a trailing attribute or comment governs
+/// something *outside* the run, which a union would displace.
+/// Group-wise dedupe in the builder relies on the same decomposition.
pub(crate) fn element_groups(tree: &Tree, run: &[NodeId]) -> Option<Vec<Vec<NodeId>>> {
let mut groups = Vec::new();
let mut current = Vec::new();
for &node in run {
current.push(node);
- if tree.is_named(node) && !tree.lang().binds_forward(tree.kind(node)) {
+ if tree.is_named(node)
+ && !tree.is_extra(node)
+ && !tree.lang().binds_forward(tree.kind(node))
+ {
groups.push(std::mem::take(&mut current));
}
}