Conversation
- `LineEntryItem` gains `group: Option<bool>` + a `group()` accessor; the builder threads `group_pending` parallel to `merge_pending` and records membership as `fuses_left: bool` on `LineItem::Segment`. - `merge = true` implies `group = true` unless `group = false`; `group` on a separator entry warns and is ignored. Spacing (merge) and color-grouping (group) stay orthogonal. - `LineItem::Segment` gains a field — a breaking change to the published crate (changeset: linesmith-core major -> 0.3.0). - Add a `#[cfg(test)] LineItem::seg()` constructor and migrate the test fixtures to it, retiring the per-field-add literal churn. The group-lead color render pass that consumes `fuses_left` is deferred to lsm-p0p2; until it lands `group` is parsed and grouped but not yet colored. Specs and the config schema updated to match. lsm-v6f0
There was a problem hiding this comment.
Pull request overview
Implements the config + builder plumbing for ADR-0029’s group color-grouping flag by adding a typed group: Option<bool> to [line].segments items and threading grouping state through the segment builder as a new fuses_left: bool carrier on LineItem::Segment (render-side consumption deferred).
Changes:
- Add
group: Option<bool>toLineEntryItem+LineEntry::group()accessor; regenerate JSON schema and update specs to mark config/builder as implemented. - Extend the segment builder to track
group_pendingalongsidemerge_pendingand emitLineItem::Segment { fuses_left: ... }. - Update tests/fixtures to use a centralized
LineItem::seg()helper (test-only) and add new coverage for the(merge, group)behavior matrix and chaining/persistence rules.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/specs/segment-system.md | Updates spec to reflect that group parsing + builder grouping are now implemented; render pass still pending. |
| docs/specs/config.md | Updates config spec comments and validation rule text for implemented group behavior. |
| crates/linesmith-core/src/segments/mod.rs | Adds fuses_left to LineItem::Segment, updates Debug impl, adds LineItem::seg() test helper. |
| crates/linesmith-core/src/segments/builder/dispatch.rs | Threads group_pending through build_one_line and sets fuses_left on emitted segments; warns on group set on separator entries. |
| crates/linesmith-core/src/segments/builder/tests.rs | Migrates fixtures to LineItem::seg() and adds focused tests for grouping semantics and warning behavior. |
| crates/linesmith-core/src/layout/tests.rs | Migrates layout test fixtures to LineItem::seg(). |
| crates/linesmith-core/src/layout/mod.rs | Updates pattern match to tolerate added LineItem::Segment field (..). |
| crates/linesmith-core/src/config.rs | Adds typed group field and LineEntry::group() accessor; updates malformed-entry fallback initialization. |
| config.schema.json | Regenerates schema to include group under [line].segments inline-table entries. |
| .changeset/group-flag-fuses-left.md | Records the breaking change (new public field on LineItem::Segment) and describes intended semantics. |
| .beads/issues.jsonl | Closes tracking bead lsm-v6f0 with implementation notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the config + builder half of ADR-0029's
groupcolor-grouping flag (beadlsm-v6f0). The render-side color pass that consumes the grouping is deferred tolsm-p0p2.[line].segmentsentries gaingroup: Option<bool>(onLineEntryItem) plus aLineEntry::group()accessor mirroringmerge().groupis orthogonal tomerge:mergecontrols spacing (separator vs abut),groupcontrols color (fuse vs divide).build_one_linethreads agroup_pendingflag parallel tomerge_pendingand records membership as a newfuses_left: boolonLineItem::Segment(true when a segment shares a color group with the one to its left; a maximal run + its leftmost lead = one group).merge = trueimpliesgroup = trueunlessgroup = falseoverrides;groupon a separator entry warns and is ignored.LineItem::Segmentgains a field. The variant is public and not#[non_exhaustive], so downstream constructors/exhaustive matches break (add the field /..). Changeset:linesmith-coremajor → 0.3.0.(merge, group)states, 3+ chaining, drop-carry, cross-separator persistence (the dogfood case), and warn-on-separator. Also adds a#[cfg(test)] LineItem::seg()constructor and migrates ~27 fixture literals to it, retiring the per-field-add churn.config.schema.json; updatedconfig.md/segment-system.mdmarkers (now "implemented; color rendering pending lsm-p0p2").Reviewed across two full cycles (Codex + code-reviewer + pr-test-analyzer + type-design-analyzer on the feature; Codex + code-reviewer on the refactor) — spec-conformance Verdict YES, maintainability Approve. All gates green:
mise run check+ 1254 tests.