Skip to content

fix(merge): drop a Required heading the driver empties - #323

Open
kixelated wants to merge 3 commits into
mainfrom
quest/m0/required-heading
Open

kixelated wants to merge 3 commits into
mainfrom
quest/m0/required-heading

Conversation

@kixelated

@kixelated kixelated commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/required-heading.md: when the quest resolve driver's list merge empties a Required list, the ## Required heading goes too, so the merged tree passes quest check (the moq-dev/moq#7251 failure).

Changes

  • src/core/merge.ts: resolveLists notes each conflict it resolves to an empty Required list, then dropEmptyRequired removes that section's heading and blank body when the section holds nothing else and no conflict remains in it. A blank line the removal leaves doubled goes too, as quest rm (drop in src/core/edit.ts) does. Item now carries its section so the resolution knows which list it emptied.
  • test/resolve.test.ts: both sides each removing one of the last two entries, with Required last and before ## Related; beside a prose conflict left unresolved; a conflict left inside the section (including one with no ours lines) keeps the heading; an emptied Related keeps its heading and quest check reports it.
  • Deletes the quest and its link from quest/m0/README.md.

Choices made without asking

  • Only Required is dropped, per the quest's Plan; emptied Related/Closes stay for quest check, which already reports them (src/core/rules.ts).
  • The heading is dropped only when this merge's resolution emptied the section, not for a Required that was already empty on the inputs.
  • Kept the removal in merge.ts rather than sharing drop from edit.ts: the merge works on chunks with conflicts interleaved, which drop's whole-text interface does not fit.

Checks

  • just check: pass
  • just test (via quest gate): 726 passed, 2 skipped; 426 passed
  • quest check --base origin/main: 69 documents ok

Review

  • Reviewer: Codex, gpt-6.1-sol, medium effort (quest review), session 01a19290-0251-7c53-ba00-a735cc26b575
  • Reviewed head: a7f3e39, round 2: no findings
  • Round 1 (head 1245cf0) finding, fixed: a remaining conflict with no ours lines inside the section let the heading drop; every remaining conflict's position is now tracked, with a regression test.

Follow-ups

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 3 commits October 31, 2026 08:11
When the quest resolve driver's list merge leaves a Required section with
nothing but blank lines, drop the heading with it, as quest rm does, so the
merged tree passes quest check. Other empty list sections stay for quest
check to report.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A conflict whose ours side has no lines left no trace in the blank-body
check, so the heading went even though resolving that conflict could put an
entry back under it. Track every remaining conflict's position instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title The merge driver drops a Required heading it empties fix(merge): drop a Required heading the driver empties Oct 31, 2026
@kixelated
kixelated marked this pull request as ready for review October 31, 2026 15:29
@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0e80284d-8cbb-47fe-862d-3fa86002c3df
Commits

Reviewing files that changed from the base of the PR and between 66a39b7 and a7f3e39.

Files selected for processing: 4
  • quest/m0/README.md
  • quest/m0/required-heading.md
  • src/core/merge.ts
  • test/resolve.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

This branch has not been deployed

No deployments
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