From f35bede9dd40e203c85ce457008cee222d54e6a6 Mon Sep 17 00:00:00 2001 From: Frank O'Hara Date: Sun, 9 Aug 2026 08:39:31 -0600 Subject: [PATCH] fix(add): count annotation lines from the source, not from parsed metadata Appending after a task that carried a hand-written `@depends-on: a, b, c` put the new task two lines too far down, outside the `## Tasks` section entirely. In the repro the Notes heading ended up separated from its own prose. `count_annotation_lines` added one line per parsed dependency. That holds for what `MarkdownEmitter::format_task_annotations` writes, one reference per line, but the parser splits a comma-separated value into several references off a single line, so the count ran two lines long. `@labels` had the same problem from the other direction and was simply skipped, with a comment saying it could not be counted, because labels can be inline on the checkbox line or in an `@labels:` block and the parsed metadata does not say which. Both are the same defect: the number of source lines cannot be recovered from parsed metadata, because the same metadata can be written in more than one shape. So record it at parse time instead. The parser already collects the annotation lines for a task and therefore already knows the count; it now keeps it on `Task::annotation_line_count`, and the resolver uses that. This settles the comma form, `@labels`, and any future annotation with more than one written shape, without the resolver having to know about any of them. Tasks built in memory rather than parsed (the TUI's in-memory tasks, test fixtures) have no source lines to have counted, so `annotation_line_count` is 0 and the resolver falls back to deriving a count from the metadata, assuming the emitter's shape. That keeps existing behaviour for callers that never touch a file. Tests: three integration tests appending after a task with comma-form `@depends-on`, with one `@depends-on` per line, and with block-form `@labels`, each asserting the new task lands inside `## Tasks` and the following section keeps its content. Two unit tests covering the recorded count winning over the derived estimate, and the fallback for an unparsed task. The comma-form and block-labels tests fail without the fix. Also files two `lash format` bugs found while verifying: it deletes every section after `## Tasks`, and it duplicates inline labels on every run so the output never reaches a fixed point. --- crates/lash-cli/src/commands/complete.rs | 1 + crates/lash-cli/src/utils/task_target.rs | 1 + crates/lash-cli/tests/add_command_test.rs | 140 ++++++++++++++++++ crates/lash-core/src/creation/placement.rs | 117 ++++++++++++--- crates/lash-core/src/formatter/mod.rs | 1 + crates/lash-core/src/linter/linter.rs | 1 + .../linter/rules/crossfile/circular_deps.rs | 1 + .../rules/crossfile/dependency_exists.rs | 11 ++ .../linter/rules/crossfile/index_file_refs.rs | 3 + .../linter/rules/crossfile/orphaned_files.rs | 1 + .../rules/crossfile/valid_path_resolution.rs | 2 + .../src/linter/rules/semantic/auto_waive.rs | 1 + .../src/linter/rules/semantic/duplicate_id.rs | 1 + .../src/linter/rules/semantic/empty_title.rs | 1 + .../rules/semantic/status_consistency.rs | 1 + .../linter/rules/semantic/valid_estimate.rs | 1 + .../src/linter/rules/semantic/valid_label.rs | 1 + .../src/linter/rules/semantic/valid_owner.rs | 1 + .../linter/rules/syntax/checkbox_pattern.rs | 1 + .../src/linter/rules/syntax/depth_limit.rs | 1 + crates/lash-core/src/parser/builder.rs | 1 + crates/lash-core/src/parser/checkbox.rs | 8 + crates/lash-core/src/parser/mod.rs | 2 + .../tests/linter_integration_tests.rs | 1 + crates/lash-db/src/dependency_updater.rs | 1 + crates/lash-db/src/graph_builder.rs | 1 + crates/lash-db/src/repository/dependencies.rs | 1 + crates/lash-db/src/repository/doc_ref.rs | 1 + crates/lash-db/src/repository/labels.rs | 1 + crates/lash-db/src/repository/tasks.rs | 1 + .../lash-db/tests/graph_integration_tests.rs | 1 + crates/lash-types/src/task.rs | 15 ++ lash.index.md | 48 +++++- 33 files changed, 345 insertions(+), 25 deletions(-) diff --git a/crates/lash-cli/src/commands/complete.rs b/crates/lash-cli/src/commands/complete.rs index a7120b8..b019b3c 100644 --- a/crates/lash-cli/src/commands/complete.rs +++ b/crates/lash-cli/src/commands/complete.rs @@ -628,6 +628,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata { depends_on: deps .iter() diff --git a/crates/lash-cli/src/utils/task_target.rs b/crates/lash-cli/src/utils/task_target.rs index 5bef66a..8c72928 100644 --- a/crates/lash-cli/src/utils/task_target.rs +++ b/crates/lash-cli/src/utils/task_target.rs @@ -86,6 +86,7 @@ mod tests { parent_id: None, order_index: order, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-cli/tests/add_command_test.rs b/crates/lash-cli/tests/add_command_test.rs index aea0d75..3cd25a7 100644 --- a/crates/lash-cli/tests/add_command_test.rs +++ b/crates/lash-cli/tests/add_command_test.rs @@ -654,3 +654,143 @@ fn test_add_rejects_an_agent_note_line_starting_with_an_annotation() { let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); assert!(!content.contains("At note task")); } + +// --------------------------------------------------------------------- +// `@depends-on` written in comma form +// +// `count_annotation_lines` added one line per parsed dependency, which +// holds for what the emitter writes but not for a hand-written +// `@depends-on: a, b, c`. The parser splits that single line into three +// references, so the count ran two lines long and the insertion point +// escaped the task block, landing the new task outside `## Tasks`. +// --------------------------------------------------------------------- + +#[test] +fn test_add_after_comma_form_depends_on_stays_inside_the_tasks_section() { + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "tasks.md", + r#"# Tasks + +@id: tasks + +## Tasks + +- [ ] Alpha +- [ ] Beta +- [ ] Gamma +- [ ] Last task + @depends-on: alpha, beta, gamma + +## Notes + +Prose that must stay under its heading. +"#, + ) + .build(); + index(&project); + + add_task(&project, "Second task", "tasks.md"); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + + let task = task_line_number(&content, "- [ ] Second task"); + assert!( + task > task_line_number(&content, "@depends-on: alpha, beta, gamma"), + "task must land after the dependency line:\n{content}" + ); + assert!( + task < task_line_number(&content, "## Notes"), + "task escaped the Tasks section:\n{content}" + ); + assert!( + content.contains("## Notes\n\nProse that must stay under its heading."), + "the Notes heading was split from its prose:\n{content}" + ); +} + +#[test] +fn test_add_after_one_dependency_per_line_still_clears_the_block() { + // The other half of the ambiguity: the same three references written the + // way the emitter writes them occupy three lines, not one. + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "tasks.md", + r#"# Tasks + +@id: tasks + +## Tasks + +- [ ] Alpha +- [ ] Beta +- [ ] Gamma +- [ ] Last task + @depends-on: alpha + @depends-on: beta + @depends-on: gamma + +## Notes + +Prose that must stay under its heading. +"#, + ) + .build(); + index(&project); + + add_task(&project, "Second task", "tasks.md"); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + + let task = task_line_number(&content, "- [ ] Second task"); + assert!( + task > task_line_number(&content, "@depends-on: gamma"), + "task was spliced into the dependency block:\n{content}" + ); + assert!( + task < task_line_number(&content, "## Notes"), + "task escaped the Tasks section:\n{content}" + ); +} + +#[test] +fn test_add_after_a_task_whose_labels_use_the_block_form() { + // `@labels:` used to be uncountable for the same reason, since labels can + // also live inline on the checkbox line. + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "tasks.md", + r#"# Tasks + +@id: tasks + +## Tasks + +- [ ] Last task + @labels: backend, infra + +## Notes + +Prose that must stay under its heading. +"#, + ) + .build(); + index(&project); + + add_task(&project, "Second task", "tasks.md"); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + + let task = task_line_number(&content, "- [ ] Second task"); + assert!( + task > task_line_number(&content, "@labels: backend, infra"), + "task was spliced above the labels line:\n{content}" + ); + assert!( + task < task_line_number(&content, "## Notes"), + "task escaped the Tasks section:\n{content}" + ); +} diff --git a/crates/lash-core/src/creation/placement.rs b/crates/lash-core/src/creation/placement.rs index 9f84637..fd82f9e 100644 --- a/crates/lash-core/src/creation/placement.rs +++ b/crates/lash-core/src/creation/placement.rs @@ -279,27 +279,31 @@ impl PlacementResolver { /// Count the number of annotation lines that follow a task /// - /// Every annotation type is written on its own line after the task - /// checkbox line, so appending correctly requires knowing exactly how - /// many trailing lines belong to the task being appended after — - /// undercounting here inserts the new task in the middle of that - /// annotation block, silently detaching a trailing annotation (e.g. an - /// existing task's `@id:`) from its owner. + /// Appending after a task means stepping past its whole block: the + /// checkbox line plus every annotation line below it. Getting this wrong + /// in either direction is a bug. Undercounting splices the new task into + /// the middle of the annotation block, detaching a trailing annotation + /// from its owner or destroying the continuation lines of a multi-line + /// note. Overcounting pushes the insertion point past the end of the + /// block, which can land the new task outside the `## Tasks` section + /// entirely. /// - /// An annotation value may span several source lines: the parser folds - /// indented continuation lines into the value, joining them with `\n` - /// (see `parser::annotations::parse_annotation_block`). Such a value - /// therefore occupies `value.lines().count()` source lines, not one, and - /// counting it as one splices the new task into the middle of the note — - /// destroying the continuation lines on the next reindex. + /// For a task that came from a file, the count is whatever the parser + /// counted, recorded on [`Task::annotation_line_count`]. It is the only + /// place with the answer, because the same metadata can be written in more + /// than one shape and the parsed [`lash_types::task::TaskMetadata`] does + /// not record which was used: /// - /// `@labels` is deliberately not counted: labels may be written inline - /// on the checkbox line (`#backend`) or as a separate `@labels:` block, - /// and [`lash_types::task::TaskMetadata`] doesn't record which form was - /// used, so it can't be counted without re-reading the source line. - /// `lash add` itself always emits labels inline (see - /// `MarkdownEmitter::format_task_line`), so this only affects appending - /// after a hand-edited task that used the block form. + /// - `@depends-on: a, b, c` on one line, or one `@depends-on:` line each + /// - `#backend` inline on the checkbox line, or an `@labels:` block + /// - a value folded across several indented continuation lines + /// + /// For a task built programmatically rather than parsed (the TUI's + /// in-memory tasks, test fixtures), there are no source lines to have + /// counted, so the count is derived from the metadata on the assumption + /// that it will be written the way [`super::emitter::MarkdownEmitter`] + /// writes it: one line per annotation, one line per `@depends-on` and + /// `@doc` reference, labels inline. fn count_annotation_lines(task: &Task) -> usize { /// Source lines occupied by a single annotation value. /// @@ -309,6 +313,10 @@ impl PlacementResolver { value.lines().count().max(1) } + if task.annotation_line_count > 0 { + return task.annotation_line_count; + } + let mut count = 0; // @id: one line, only when it's an explicit annotation (not a @@ -330,11 +338,7 @@ impl PlacementResolver { } // @depends-on / @doc: one line per reference, as emitted by - // `MarkdownEmitter::format_task_annotations`. A hand-written - // comma-separated `@depends-on: a, b, c` parses to several references - // from a single line and is over-counted here; that is the same - // inline-vs-block ambiguity described above for `@labels`, and - // over-counting appends after the block rather than inside it. + // `MarkdownEmitter::format_task_annotations`. count += task.metadata.depends_on.len(); count += task.metadata.docs.len(); @@ -982,6 +986,71 @@ mod tests { assert_eq!(placement.order_index, 3); } + /// An explicit-id dependency reference, the form `lash add --depends-on` + /// produces. + fn dep_ref(target: &str) -> lash_types::DependencyRef { + lash_types::DependencyRef::new(target.to_string(), lash_types::DependencyKind::ExplicitId) + } + + #[test] + fn test_parsed_annotation_line_count_wins_over_the_derived_estimate() { + // A hand-written `@depends-on: a, b, c` parses to three references off + // a single line. Deriving the count from the metadata says three + // lines, which pushes the insertion point two lines past the end of + // the task block; the parser's own count says one. + let config = ConfigBuilder::new().build().unwrap(); + let validator = TaskValidator::new(config); + + let mut task = TaskBuilder::new("Existing task") + .id("existing") + .order_index(0) + .line_number(6) + .build() + .unwrap(); + task.metadata.depends_on = ["a", "b", "c"].map(dep_ref).to_vec(); + // `@id:` on line 7, `@depends-on: a, b, c` on line 8. + task.annotation_line_count = 2; + + let mut tasks = TaskTree::new(); + tasks.add_task(task).unwrap(); + let file = create_test_file(tasks); + + let request = TaskCreationRequestBuilder::new("New task").build(); + let ctx = validator.validate(&request, Some(&file)).unwrap(); + let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); + + assert_eq!(placement.anchor.line(), Some(9)); + } + + #[test] + fn test_unparsed_task_falls_back_to_deriving_the_count() { + // Tasks built in memory rather than parsed have no source lines to + // have counted, so the derived estimate is all there is. It assumes + // the emitter's shape: one line per reference. + let config = ConfigBuilder::new().build().unwrap(); + let validator = TaskValidator::new(config); + + let mut task = TaskBuilder::new("Existing task") + .id("existing") + .order_index(0) + .line_number(6) + .build() + .unwrap(); + task.metadata.depends_on = ["a", "b"].map(dep_ref).to_vec(); + assert_eq!(task.annotation_line_count, 0); + + let mut tasks = TaskTree::new(); + tasks.add_task(task).unwrap(); + let file = create_test_file(tasks); + + let request = TaskCreationRequestBuilder::new("New task").build(); + let ctx = validator.validate(&request, Some(&file)).unwrap(); + let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); + + // Checkbox on 6, `@id:` on 7, two `@depends-on:` lines on 8 and 9. + assert_eq!(placement.anchor.line(), Some(10)); + } + #[test] fn test_append_after_task_with_explicit_id_skips_its_annotation_line() { // Regression test: appending after a task that carries an explicit diff --git a/crates/lash-core/src/formatter/mod.rs b/crates/lash-core/src/formatter/mod.rs index 08f8e22..ca6d4c3 100644 --- a/crates/lash-core/src/formatter/mod.rs +++ b/crates/lash-core/src/formatter/mod.rs @@ -742,6 +742,7 @@ mod tests { parent_id: parent.map(std::string::ToString::to_string), order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/linter.rs b/crates/lash-core/src/linter/linter.rs index 01df8d8..87d087a 100644 --- a/crates/lash-core/src/linter/linter.rs +++ b/crates/lash-core/src/linter/linter.rs @@ -311,6 +311,7 @@ mod tests { parent_id: None, order_index: i, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/crossfile/circular_deps.rs b/crates/lash-core/src/linter/rules/crossfile/circular_deps.rs index f1f73da..2532edf 100644 --- a/crates/lash-core/src/linter/rules/crossfile/circular_deps.rs +++ b/crates/lash-core/src/linter/rules/crossfile/circular_deps.rs @@ -270,6 +270,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/crossfile/dependency_exists.rs b/crates/lash-core/src/linter/rules/crossfile/dependency_exists.rs index 3e82c19..fe4cf6b 100644 --- a/crates/lash-core/src/linter/rules/crossfile/dependency_exists.rs +++ b/crates/lash-core/src/linter/rules/crossfile/dependency_exists.rs @@ -200,6 +200,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -218,6 +219,7 @@ mod tests { parent_id: None, order_index: i, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), @@ -309,6 +311,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -348,6 +351,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -389,6 +393,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -428,6 +433,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -464,6 +470,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -499,6 +506,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -544,6 +552,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -590,6 +599,7 @@ mod tests { parent_id: None, order_index: 1, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -715,6 +725,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/crossfile/index_file_refs.rs b/crates/lash-core/src/linter/rules/crossfile/index_file_refs.rs index e9e5e6e..dc788a1 100644 --- a/crates/lash-core/src/linter/rules/crossfile/index_file_refs.rs +++ b/crates/lash-core/src/linter/rules/crossfile/index_file_refs.rs @@ -161,6 +161,7 @@ mod tests { parent_id: None, order_index: i, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), @@ -379,6 +380,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), @@ -392,6 +394,7 @@ mod tests { parent_id: None, order_index: 1, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs b/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs index 1bc106c..65905f6 100644 --- a/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs +++ b/crates/lash-core/src/linter/rules/crossfile/orphaned_files.rs @@ -301,6 +301,7 @@ mod tests { parent_id: None, order_index: i, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/crossfile/valid_path_resolution.rs b/crates/lash-core/src/linter/rules/crossfile/valid_path_resolution.rs index abacff2..6b20967 100644 --- a/crates/lash-core/src/linter/rules/crossfile/valid_path_resolution.rs +++ b/crates/lash-core/src/linter/rules/crossfile/valid_path_resolution.rs @@ -236,6 +236,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), @@ -485,6 +486,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata, body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/semantic/auto_waive.rs b/crates/lash-core/src/linter/rules/semantic/auto_waive.rs index c35f87a..576d1f1 100644 --- a/crates/lash-core/src/linter/rules/semantic/auto_waive.rs +++ b/crates/lash-core/src/linter/rules/semantic/auto_waive.rs @@ -188,6 +188,7 @@ mod tests { parent_id: parent.map(std::string::ToString::to_string), order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/semantic/duplicate_id.rs b/crates/lash-core/src/linter/rules/semantic/duplicate_id.rs index 9a2fc80..d3566bb 100644 --- a/crates/lash-core/src/linter/rules/semantic/duplicate_id.rs +++ b/crates/lash-core/src/linter/rules/semantic/duplicate_id.rs @@ -144,6 +144,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/semantic/empty_title.rs b/crates/lash-core/src/linter/rules/semantic/empty_title.rs index 1cc5dfb..b72f41c 100644 --- a/crates/lash-core/src/linter/rules/semantic/empty_title.rs +++ b/crates/lash-core/src/linter/rules/semantic/empty_title.rs @@ -120,6 +120,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/semantic/status_consistency.rs b/crates/lash-core/src/linter/rules/semantic/status_consistency.rs index 1335d8f..4e3632a 100644 --- a/crates/lash-core/src/linter/rules/semantic/status_consistency.rs +++ b/crates/lash-core/src/linter/rules/semantic/status_consistency.rs @@ -173,6 +173,7 @@ mod tests { parent_id: parent.map(std::string::ToString::to_string), order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/semantic/valid_estimate.rs b/crates/lash-core/src/linter/rules/semantic/valid_estimate.rs index 7f326e7..4627d42 100644 --- a/crates/lash-core/src/linter/rules/semantic/valid_estimate.rs +++ b/crates/lash-core/src/linter/rules/semantic/valid_estimate.rs @@ -151,6 +151,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata { estimate: estimate.map(std::string::ToString::to_string), ..Default::default() diff --git a/crates/lash-core/src/linter/rules/semantic/valid_label.rs b/crates/lash-core/src/linter/rules/semantic/valid_label.rs index be6902e..5d3acf5 100644 --- a/crates/lash-core/src/linter/rules/semantic/valid_label.rs +++ b/crates/lash-core/src/linter/rules/semantic/valid_label.rs @@ -164,6 +164,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata { labels: labels.iter().map(|s| (*s).to_string()).collect(), ..Default::default() diff --git a/crates/lash-core/src/linter/rules/semantic/valid_owner.rs b/crates/lash-core/src/linter/rules/semantic/valid_owner.rs index adc3c78..ca9adb0 100644 --- a/crates/lash-core/src/linter/rules/semantic/valid_owner.rs +++ b/crates/lash-core/src/linter/rules/semantic/valid_owner.rs @@ -170,6 +170,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata { owner: owner.map(std::string::ToString::to_string), ..Default::default() diff --git a/crates/lash-core/src/linter/rules/syntax/checkbox_pattern.rs b/crates/lash-core/src/linter/rules/syntax/checkbox_pattern.rs index 4ed5b01..7572cd1 100644 --- a/crates/lash-core/src/linter/rules/syntax/checkbox_pattern.rs +++ b/crates/lash-core/src/linter/rules/syntax/checkbox_pattern.rs @@ -154,6 +154,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/linter/rules/syntax/depth_limit.rs b/crates/lash-core/src/linter/rules/syntax/depth_limit.rs index 0a14f7d..e843f19 100644 --- a/crates/lash-core/src/linter/rules/syntax/depth_limit.rs +++ b/crates/lash-core/src/linter/rules/syntax/depth_limit.rs @@ -120,6 +120,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/parser/builder.rs b/crates/lash-core/src/parser/builder.rs index a393c44..4c6ce38 100644 --- a/crates/lash-core/src/parser/builder.rs +++ b/crates/lash-core/src/parser/builder.rs @@ -220,6 +220,7 @@ impl TaskTreeBuilder { parent_id, order_index, line_number: line.line_num, + annotation_line_count: line.annotation_line_count, metadata, body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-core/src/parser/checkbox.rs b/crates/lash-core/src/parser/checkbox.rs index 3bdfc47..e875dc7 100644 --- a/crates/lash-core/src/parser/checkbox.rs +++ b/crates/lash-core/src/parser/checkbox.rs @@ -47,6 +47,13 @@ pub struct CheckboxLine { /// Annotation block following the task line (if any) pub annotations: Option, + + /// Number of source lines the annotation block occupies + /// + /// Tracked separately from `annotations` because a single annotation may + /// be written across several lines, and several annotations may share one + /// line, so the parsed block does not imply a line count. + pub annotation_line_count: usize, } impl CheckboxLine { @@ -80,6 +87,7 @@ impl CheckboxLine { line_num, column, annotations: None, + annotation_line_count: 0, } } diff --git a/crates/lash-core/src/parser/mod.rs b/crates/lash-core/src/parser/mod.rs index 70cf19c..b28adfc 100644 --- a/crates/lash-core/src/parser/mod.rs +++ b/crates/lash-core/src/parser/mod.rs @@ -536,12 +536,14 @@ fn parse_task_section_internal(content: &str, ctx: &mut ParseContext) -> ParseRe if annotation_lines.is_empty() { i += 1; } else { + let annotation_line_count = annotation_lines.len(); match annotations::parse_annotation_block( annotation_lines.into_iter(), Some(ctx.config), ) { Ok(block) => { cb_line.annotations = Some(block); + cb_line.annotation_line_count = annotation_line_count; i = j; // Skip past the annotation lines we consumed } Err(e) => { diff --git a/crates/lash-core/tests/linter_integration_tests.rs b/crates/lash-core/tests/linter_integration_tests.rs index f94c2d5..68c88be 100644 --- a/crates/lash-core/tests/linter_integration_tests.rs +++ b/crates/lash-core/tests/linter_integration_tests.rs @@ -55,6 +55,7 @@ fn make_task(id: &str, title: &str, depth: u8) -> Task { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/dependency_updater.rs b/crates/lash-db/src/dependency_updater.rs index 3dbe968..095c458 100644 --- a/crates/lash-db/src/dependency_updater.rs +++ b/crates/lash-db/src/dependency_updater.rs @@ -400,6 +400,7 @@ mod tests { parent_id: parent_id.map(String::from), order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/graph_builder.rs b/crates/lash-db/src/graph_builder.rs index 403711f..d6671eb 100644 --- a/crates/lash-db/src/graph_builder.rs +++ b/crates/lash-db/src/graph_builder.rs @@ -208,6 +208,7 @@ mod tests { parent_id, order_index: order, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/repository/dependencies.rs b/crates/lash-db/src/repository/dependencies.rs index aa353f8..c7006cd 100644 --- a/crates/lash-db/src/repository/dependencies.rs +++ b/crates/lash-db/src/repository/dependencies.rs @@ -294,6 +294,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/repository/doc_ref.rs b/crates/lash-db/src/repository/doc_ref.rs index de2e948..6bf4c3a 100644 --- a/crates/lash-db/src/repository/doc_ref.rs +++ b/crates/lash-db/src/repository/doc_ref.rs @@ -353,6 +353,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/repository/labels.rs b/crates/lash-db/src/repository/labels.rs index 2175263..973591f 100644 --- a/crates/lash-db/src/repository/labels.rs +++ b/crates/lash-db/src/repository/labels.rs @@ -349,6 +349,7 @@ mod tests { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/src/repository/tasks.rs b/crates/lash-db/src/repository/tasks.rs index 3af2f9f..5a2dafb 100644 --- a/crates/lash-db/src/repository/tasks.rs +++ b/crates/lash-db/src/repository/tasks.rs @@ -892,6 +892,7 @@ mod tests { parent_id, order_index: order, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-db/tests/graph_integration_tests.rs b/crates/lash-db/tests/graph_integration_tests.rs index b5a098c..2c9f456 100644 --- a/crates/lash-db/tests/graph_integration_tests.rs +++ b/crates/lash-db/tests/graph_integration_tests.rs @@ -40,6 +40,7 @@ fn create_test_task( parent_id, order_index: order, line_number: 0, + annotation_line_count: 0, metadata: TaskMetadata::default(), body: None, contextual_notes: Vec::new(), diff --git a/crates/lash-types/src/task.rs b/crates/lash-types/src/task.rs index 9de6b1d..c0ae2a3 100644 --- a/crates/lash-types/src/task.rs +++ b/crates/lash-types/src/task.rs @@ -193,6 +193,18 @@ pub struct Task { #[serde(default)] pub line_number: usize, + /// Source lines the task's annotation block occupies, directly below the + /// checkbox line + /// + /// Recorded by the parser, which is the only place that can know it: the + /// same metadata can be written in more than one shape (`@depends-on: a, b` + /// on one line or one `@depends-on:` line each, labels inline on the + /// checkbox or in an `@labels:` block), so the count cannot be recovered + /// from [`TaskMetadata`] afterwards. 0 for a task that was not parsed from + /// a file. + #[serde(default)] + pub annotation_line_count: usize, + /// Optional metadata pub metadata: TaskMetadata, @@ -355,6 +367,7 @@ pub struct TaskBuilder { parent_id: Option, order_index: usize, line_number: usize, + annotation_line_count: usize, id: Option, has_explicit_id: bool, metadata: TaskMetadata, @@ -373,6 +386,7 @@ impl TaskBuilder { parent_id: None, order_index: 0, line_number: 0, + annotation_line_count: 0, id: None, has_explicit_id: false, metadata: TaskMetadata::default(), @@ -506,6 +520,7 @@ impl TaskBuilder { parent_id: self.parent_id, order_index: self.order_index, line_number: self.line_number, + annotation_line_count: self.annotation_line_count, metadata: self.metadata, body: self.body, contextual_notes: self.contextual_notes, diff --git a/lash.index.md b/lash.index.md index 4b98416..463a1ac 100644 --- a/lash.index.md +++ b/lash.index.md @@ -122,7 +122,7 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. point `[coverage] command` at a tool the image has; make the paths portable rather than machine-specific. Verify by running `flawd run` with default isolation and confirming per-test collection succeeds instead of falling back -- [ ] `@depends-on` comma form over-counts lines, appending outside the tasks section #cli #bug +- [x] `@depends-on` comma form over-counts lines, appending outside the tasks section #cli #bug - Follow-up to the multi-line note fix, same function (`PlacementResolver::count_annotation_lines`). `count += depends_on.len()` assumes one source line per reference, which holds for what @@ -147,6 +147,14 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. content, so it needs plumbing - Regression test: append after a task with comma-separated `@depends-on` followed by a section heading; assert the new task lands inside `## Tasks` + - Fixed the way the ticket called for: the parser now records how many source + lines a task's annotation block occupies, on `Task::annotation_line_count`, + and the resolver uses that instead of re-deriving a count from the parsed + metadata. This settles `@labels` too, which was previously uncountable for + the same reason, and any future annotation that can be written in more than + one shape. Tasks built in memory rather than parsed (the TUI, test + fixtures) have no source lines to have counted, so they keep the derived + estimate - [x] `lash add --agent-note` with an embedded newline writes a malformed file and loses content #cli #bug #data-loss - Latent counterpart to the multi-line note fix, on the write side. `MarkdownEmitter::format_task_annotations` builds the note with @@ -207,6 +215,44 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. reported line now comes from where the task was actually written, so the `:0` in the success message is gone too, as is the missing trailing newline noted under the ID ticket below +- [ ] `lash format` deletes every section after `## Tasks` #cli #bug #data-loss #launch-blocker + - Worse than the `lash add` bugs it was found next to, because `format` is + documented as a normalizer and the README tells people to run it. Found + 2026-08-09 while checking that the empty-Tasks-section fix left files + lint-clean + - Repro: a file with `## Tasks`, then `## Notes` with prose, then + `## References` with a link. `lash format` on it emits the header and the + Tasks section and nothing else. Notes, References and all their content are + gone. Exit code 0, no warning + - This repo's own `lash.index.md` has a `## Description` section that sits + *before* `## Tasks` and so survives, which is probably why nobody hit it + - Cause is in the emit half of the formatter: it reconstructs the file from + the parsed model, and the model keeps the header, the description and the + task tree but nothing about sections it does not understand. Anything the + parser did not model is not written back + - Fix: the formatter must preserve source it does not model. Either carry the + unmodelled tail through the parse (a raw span per unrecognized section) or + have the formatter rewrite in place, editing only the lines it owns rather + than regenerating the file + - Regression tests: format a file with sections before and after `## Tasks` + and assert byte-exact preservation of both; a file with a trailing section + containing a fenced code block; and a round-trip property test asserting + `format(format(x)) == format(x)` +- [ ] `lash format` duplicates inline labels on every run #cli #bug + - Not idempotent, and the file grows without bound. `- [ ] task one #docs + #infra` becomes `#docs #infra #docs #infra` after one run and + `#docs #infra #docs #infra #docs #infra` after two + - The parser records inline labels in `metadata.labels` while leaving them in + the title text, so the emitter writes the title (labels included) and then + appends the labels again. Same double-counting shape as the annotation-line + bugs: one piece of information recorded in two places with no note of which + is authoritative + - `lash format --check` reports a file as needing formatting forever, since + the output never reaches a fixed point + - Fix: strip inline labels from the stored title, or have the emitter write + the title verbatim and skip the label pass. Whichever, add the + `format(format(x)) == format(x)` property test above; it catches this and + the section-deletion bug at once - [ ] `lash add` reports a task ID that does not match the indexed one #cli #bug #ux - The ID printed on creation cannot be used with `lash show` / `lash complete` / `@depends-on`, so any workflow that copies it fails. Observed repeatedly