From 727f2dfa613f2e4f6407fcabb30473427254c67a Mon Sep 17 00:00:00 2001 From: Frank O'Hara Date: Sun, 9 Aug 2026 08:03:48 -0600 Subject: [PATCH] fix(add): stop prepending above the H1 when the Tasks section is empty `lash add` against an existing file whose `## Tasks` section held no tasks wrote the checkbox at line 1, above the H1. The parser never saw it as a task, so `lash index` reported 0 tasks and `lash lint` passed clean: the task was on disk and nowhere else, with exit code 0 and a normal success message pointing at `path:0`. `resolve_append` returned `line_number: 0` whenever the file had no tasks, commented "Signal for new file", and the emitter mapped 0 to insert index 0. The sentinel conflated "brand-new file" with "existing file whose Tasks section is empty", and only the first is safe to write at offset 0. `PlacementInfo` now carries an `InsertAnchor` instead of a bare line number: either a concrete `Line(n)` or `EndOfTasksSection`. The ambiguity cannot recur, because there is no longer a number that means something other than a line. Resolving `EndOfTasksSection` needs the source text. A parsed `TaskFile` records task line numbers and nothing about section boundaries, so an empty section has nothing to anchor to, which is why the old code fell back to a hardcoded guess of line 15. The emitter resolves it instead, against the content it already reads, via a new `parser::header::tasks_section_body`. That returns the section's line span using pulldown-cmark, so a `##` inside a code fence does not close the section, and H3 headings stay inside it (files group tasks under `### Subsection` headings). With no `## Tasks` heading at all it appends at end of file rather than guessing. The line reported on success now comes from where the task was actually written, for new files as well, so `path:0` is gone. Two things fixed in passing, both in code this commit rewrites: `find_end_of_tasks_section` and its line-15 estimate are deleted rather than ported, and `insert_into_existing` restores the trailing newline that `join` dropped, which had every `lash add` leaving the file ending mid-line. Tests: five integration tests in add_command_test.rs (empty section lands below the header and reports the right line, is indexed afterwards, a following `## Notes` section survives intact, the new-file path still writes a well-formed file, and files end in a newline) plus six unit tests on anchor resolution covering empty sections, trailing blank lines, a heading inside a code fence, a file with no Tasks section, and clamping. Four of the five integration tests fail without the fix. --- crates/lash-cli/tests/add_command_test.rs | 191 +++++++++++++++++++++ crates/lash-core/src/creation/emitter.rs | 154 +++++++++++++---- crates/lash-core/src/creation/mod.rs | 2 +- crates/lash-core/src/creation/placement.rs | 121 ++++++------- crates/lash-core/src/parser/header.rs | 72 ++++++++ lash.index.md | 11 +- 6 files changed, 459 insertions(+), 92 deletions(-) diff --git a/crates/lash-cli/tests/add_command_test.rs b/crates/lash-cli/tests/add_command_test.rs index cf9626d..dd41608 100644 --- a/crates/lash-cli/tests/add_command_test.rs +++ b/crates/lash-cli/tests/add_command_test.rs @@ -366,3 +366,194 @@ fn test_add_preserves_multiline_agent_note_through_reindex() { ); } } + +// --------------------------------------------------------------------- +// Appending to a file whose `## Tasks` section is empty +// +// `resolve_append` used to return the magic line number 0 whenever the +// target file held no tasks, meaning "this is a brand-new file, write at +// offset 0". For a file that already existed with an empty `## Tasks` +// section that was catastrophically wrong: the checkbox was prepended +// above the H1, so the parser never saw it as a task, `lash index` found +// nothing, and `lash lint` still passed. The task was on disk and +// invisible. +// --------------------------------------------------------------------- + +/// Line the task line lands on, 1-indexed. +fn task_line_number(content: &str, needle: &str) -> usize { + content + .lines() + .position(|line| line.contains(needle)) + .map(|idx| idx + 1) + .unwrap_or_else(|| panic!("'{needle}' not found in:\n{content}")) +} + +/// Run `lash add`, returning the command's stdout. +fn add_task(project: &TestProject, title: &str, file: &str) -> String { + let output = run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("add") + .arg(title) + .arg("--file") + .arg(file) + .assert() + .success() + .get_output() + .stdout + .clone(); + String::from_utf8(output).unwrap() +} + +#[test] +fn test_add_to_empty_tasks_section_lands_below_the_header() { + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "empty.md", + r#"# Empty + +@id: empty + +## Tasks +"#, + ) + .build(); + index(&project); + + let stdout = add_task(&project, "Ship it", "empty.md"); + + let content = fs::read_to_string(project.file_path("empty.md")).unwrap(); + + assert_eq!( + content.lines().next(), + Some("# Empty"), + "the H1 must still be the first line:\n{content}" + ); + // The old sentinel surfaced here too, as a literal `empty.md:0`. + assert!( + stdout.contains(&format!( + "empty.md:{}", + task_line_number(&content, "- [ ] Ship it") + )), + "reported line does not match where the task was written:\n{stdout}" + ); + assert!( + task_line_number(&content, "- [ ] Ship it") > task_line_number(&content, "## Tasks"), + "task must land inside the Tasks section:\n{content}" + ); +} + +#[test] +fn test_add_to_empty_tasks_section_is_indexed() { + // The destructive part of the bug: the task was written to a place the + // parser does not read, so it existed on disk and nowhere else. + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "empty.md", + r#"# Empty + +@id: empty + +## Tasks +"#, + ) + .build(); + index(&project); + + add_task(&project, "Ship it", "empty.md"); + index(&project); + + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("list") + .assert() + .success() + .stdout(predicate::str::contains("Ship it")); +} + +#[test] +fn test_add_to_empty_tasks_section_keeps_following_sections_intact() { + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .with_file( + "sections.md", + r#"# Sections + +@id: sections + +## Tasks + +## Notes + +Prose that must survive. +"#, + ) + .build(); + index(&project); + + add_task(&project, "Ship it", "sections.md"); + + let content = fs::read_to_string(project.file_path("sections.md")).unwrap(); + + assert_eq!( + content.lines().next(), + Some("# Sections"), + "the H1 must still be the first line:\n{content}" + ); + assert!( + content.contains("Prose that must survive."), + "trailing section content was destroyed:\n{content}" + ); + + let task = task_line_number(&content, "- [ ] Ship it"); + assert!( + task > task_line_number(&content, "## Tasks") + && task < task_line_number(&content, "## Notes"), + "task escaped the Tasks section:\n{content}" + ); +} + +#[test] +fn test_add_creating_a_new_file_still_writes_a_well_formed_file() { + // Guards the other half of the split: the old code used one sentinel for + // both "new file" and "existing file with an empty section". + let project = TestProject::builder() + .with_index("test-project", "Test Project") + .build(); + index(&project); + + add_task(&project, "Ship it", "fresh.md"); + + let content = fs::read_to_string(project.file_path("fresh.md")).unwrap(); + + assert!( + task_line_number(&content, "- [ ] Ship it") > task_line_number(&content, "## Tasks"), + "task must land inside the Tasks section:\n{content}" + ); + + index(&project); + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("list") + .assert() + .success() + .stdout(predicate::str::contains("Ship it")); +} + +#[test] +fn test_add_leaves_the_file_ending_in_a_newline() { + let project = project_with_tasks_file(); + index(&project); + + add_task(&project, "Second task", "tasks.md"); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + assert!( + content.ends_with('\n'), + "file ends mid-line, every later diff shows '\\ No newline at end of file':\n{content}" + ); +} diff --git a/crates/lash-core/src/creation/emitter.rs b/crates/lash-core/src/creation/emitter.rs index 7b257cc..affe13a 100644 --- a/crates/lash-core/src/creation/emitter.rs +++ b/crates/lash-core/src/creation/emitter.rs @@ -10,8 +10,9 @@ use lash_types::TaskStatus; use std::fs; use std::path::{Path, PathBuf}; -use super::placement::PlacementInfo; +use super::placement::{InsertAnchor, PlacementInfo}; use super::validation::ValidationContext; +use crate::parser::header; /// Markdown emitter for task creation /// @@ -41,7 +42,7 @@ impl MarkdownEmitter { /// # Examples /// /// ``` - /// use lash_core::creation::{MarkdownEmitter, PlacementInfo}; + /// use lash_core::creation::{InsertAnchor, MarkdownEmitter, PlacementInfo}; /// use lash_core::creation::{TaskValidator, ValidationContext}; /// use lash_types::config::ConfigBuilder; /// use lash_types::creation::{TaskCreationRequestBuilder, FileTarget}; @@ -54,7 +55,7 @@ impl MarkdownEmitter { /// # .build(); /// # let ctx = validator.validate(&request, None).unwrap(); /// let placement = PlacementInfo { - /// line_number: 5, + /// anchor: InsertAnchor::Line(5), /// order_index: 0, /// indent_level: 0, /// }; @@ -103,24 +104,20 @@ impl MarkdownEmitter { ); // Either create new file or insert into existing file - let file_path = if is_new_file { + let (file_path, line_number) = if is_new_file { Self::create_new_file(request, &task_line, &annotation_lines)? } else { // Get the file path from the resolved file in context let file_path = ctx.resolved_file.path.clone(); - Self::insert_into_existing( - &file_path, - &task_line, - &annotation_lines, - placement.line_number, - )?; - file_path + let line_number = + Self::insert_into_existing(&file_path, &task_line, &annotation_lines, placement)?; + (file_path, line_number) }; Ok(TaskCreationResult { task_id, file_path, - line_number: placement.line_number, + line_number, is_new_file, }) } @@ -135,7 +132,11 @@ impl MarkdownEmitter { /// * `file_path` - Path to the existing file /// * `task_line` - The formatted task line /// * `annotation_lines` - Additional annotation lines to insert - /// * `line_number` - Line number to insert at (1-indexed) + /// * `placement` - Where the resolver decided the task goes + /// + /// # Returns + /// + /// The 1-indexed line the task was written to. /// /// # Errors /// @@ -144,8 +145,8 @@ impl MarkdownEmitter { file_path: &Path, task_line: &str, annotation_lines: &[String], - line_number: usize, - ) -> Result<(), TaskCreationError> { + placement: &PlacementInfo, + ) -> Result { // Read the file let content = fs::read_to_string(file_path).map_err(|e| TaskCreationError::IoError { path: file_path.to_path_buf(), @@ -153,14 +154,9 @@ impl MarkdownEmitter { })?; let lines: Vec<&str> = content.lines().collect(); - let mut new_lines = Vec::new(); + let insert_idx = Self::resolve_insert_index(&content, &lines, placement.anchor); - // Calculate insertion index (line_number is 1-indexed) - let insert_idx = if line_number == 0 { - 0 - } else { - (line_number - 1).min(lines.len()) - }; + let mut new_lines = Vec::new(); // Copy lines before insertion point new_lines.extend_from_slice(&lines[..insert_idx]); @@ -178,10 +174,47 @@ impl MarkdownEmitter { new_lines.extend_from_slice(&lines[insert_idx..]); } - // Write back atomically - Self::write_file_atomic(file_path, &new_lines.join("\n"))?; + // Write back atomically. The trailing newline is restored explicitly: + // `join` alone leaves the file ending mid-line, which shows up in every + // subsequent diff as "\ No newline at end of file". + let mut updated = new_lines.join("\n"); + updated.push('\n'); + Self::write_file_atomic(file_path, &updated)?; - Ok(()) + Ok(insert_idx + 1) + } + + /// Translate a [`InsertAnchor`] into a 0-indexed position in `lines` + /// + /// [`InsertAnchor::EndOfTasksSection`] is resolved here rather than in the + /// resolver because it depends on the source text: a parsed file records + /// task line numbers but no section boundaries, so a `## Tasks` section + /// with no tasks in it has nothing to anchor to. The new task goes after + /// the section's last content line, keeping it inside `## Tasks` and below + /// the file header. + fn resolve_insert_index(content: &str, lines: &[&str], anchor: InsertAnchor) -> usize { + match anchor { + InsertAnchor::Line(line_number) => line_number.saturating_sub(1).min(lines.len()), + InsertAnchor::EndOfTasksSection => { + // No `## Tasks` heading at all: append at the end of the file + // rather than guessing where the section would have been. + let Some(body) = header::tasks_section_body(content) else { + return lines.len(); + }; + let body_end = body.end.min(lines.len()); + let last_content = lines[body.start..body_end] + .iter() + .rposition(|line| !line.trim().is_empty()); + + match last_content { + Some(offset) => body.start + offset + 1, + // Section is empty. Land on the line after the heading's + // blank separator so the file keeps its usual + // heading/blank/task shape. + None => (body.start + 1).min(body_end), + } + } + } } /// Create a new task file with proper structure @@ -196,7 +229,8 @@ impl MarkdownEmitter { /// /// # Returns /// - /// Returns the path to the newly created file. + /// Returns the path to the newly created file and the 1-indexed line the + /// task was written to. /// /// # Errors /// @@ -205,7 +239,7 @@ impl MarkdownEmitter { request: &TaskCreationRequest, task_line: &str, annotation_lines: &[String], - ) -> Result { + ) -> Result<(PathBuf, usize), TaskCreationError> { // Extract file details from FileTarget::NewFile let (path, title, description) = match &request.file_target { FileTarget::NewFile { @@ -270,6 +304,7 @@ impl MarkdownEmitter { // 4. Tasks section content.push_str("## Tasks\n"); content.push('\n'); + let task_line_number = content.lines().count() + 1; content.push_str(task_line); content.push('\n'); @@ -292,7 +327,7 @@ impl MarkdownEmitter { // Write file atomically Self::write_file_atomic(&path, &content)?; - Ok(path) + Ok((path, task_line_number)) } /// Format a single task line in Markdown @@ -708,13 +743,18 @@ mod tests { let task_line = "- [ ] New Task"; let annotation_lines = vec![]; // No task-level annotations + let placement = PlacementInfo { + anchor: InsertAnchor::Line(8), // Line 8 (after Task 1) + order_index: 1, + indent_level: 0, + }; let result = MarkdownEmitter::insert_into_existing( &file_path, task_line, &annotation_lines, - 8, // Line 8 (after Task 1) + &placement, ); - assert!(result.is_ok()); + assert_eq!(result.unwrap(), 8); // Verify content let content = fs::read_to_string(&file_path).unwrap(); @@ -753,7 +793,7 @@ mod tests { // Create placement info let placement = PlacementInfo { - line_number: 1, + anchor: InsertAnchor::Line(1), order_index: 0, indent_level: 0, }; @@ -801,7 +841,7 @@ mod tests { let ctx = validator.validate(&request, None).unwrap(); let placement = PlacementInfo { - line_number: 1, + anchor: InsertAnchor::Line(1), order_index: 0, indent_level: 0, }; @@ -833,4 +873,54 @@ mod tests { // Cleanup let _ = fs::remove_dir_all(&temp_dir); } + + /// Resolve an anchor against `content`, returning the 0-indexed position. + fn insert_index(content: &str, anchor: InsertAnchor) -> usize { + let lines: Vec<&str> = content.lines().collect(); + MarkdownEmitter::resolve_insert_index(content, &lines, anchor) + } + + #[test] + fn test_end_of_tasks_section_lands_below_an_empty_heading() { + // The bug: an existing file with an empty `## Tasks` section resolved + // to index 0, prepending the task above the H1 where the parser never + // saw it. + let content = "# T\n\n@id: t\n\n## Tasks\n"; + assert_eq!(insert_index(content, InsertAnchor::EndOfTasksSection), 5); + } + + #[test] + fn test_end_of_tasks_section_stays_above_a_following_section() { + let content = "# T\n\n@id: t\n\n## Tasks\n\n## Notes\n\nprose\n"; + // Line index 6 is `## Notes`, so the task goes just above it and keeps + // the blank separator under the Tasks heading. + assert_eq!(insert_index(content, InsertAnchor::EndOfTasksSection), 6); + } + + #[test] + fn test_end_of_tasks_section_follows_the_last_task_not_the_trailing_blanks() { + let content = "# T\n\n## Tasks\n\n- [ ] One\n\n\n## Notes\n"; + assert_eq!(insert_index(content, InsertAnchor::EndOfTasksSection), 5); + } + + #[test] + fn test_end_of_tasks_section_ignores_a_heading_inside_a_code_fence() { + let content = "# T\n\n## Tasks\n\n- [ ] One\n\n```\n## Notes\n```\n"; + // The fenced `## Notes` must not close the section, so the whole fence + // counts as section content and the task appends after it. + assert_eq!(insert_index(content, InsertAnchor::EndOfTasksSection), 9); + } + + #[test] + fn test_end_of_tasks_section_without_a_heading_appends_at_eof() { + let content = "# T\n\nJust prose, no sections.\n"; + assert_eq!(insert_index(content, InsertAnchor::EndOfTasksSection), 3); + } + + #[test] + fn test_line_anchor_is_clamped_to_the_file_length() { + let content = "# T\n\n## Tasks\n"; + assert_eq!(insert_index(content, InsertAnchor::Line(2)), 1); + assert_eq!(insert_index(content, InsertAnchor::Line(99)), 3); + } } diff --git a/crates/lash-core/src/creation/mod.rs b/crates/lash-core/src/creation/mod.rs index d98c55b..444ba1a 100644 --- a/crates/lash-core/src/creation/mod.rs +++ b/crates/lash-core/src/creation/mod.rs @@ -15,6 +15,6 @@ pub mod service; pub mod validation; pub use emitter::MarkdownEmitter; -pub use placement::{PlacementInfo, PlacementResolver}; +pub use placement::{InsertAnchor, PlacementInfo, PlacementResolver}; pub use service::TaskCreationService; pub use validation::{TaskValidator, ValidationContext}; diff --git a/crates/lash-core/src/creation/placement.rs b/crates/lash-core/src/creation/placement.rs index fdd2fed..9f84637 100644 --- a/crates/lash-core/src/creation/placement.rs +++ b/crates/lash-core/src/creation/placement.rs @@ -10,16 +10,48 @@ use lash_types::task::Task; use super::validation::ValidationContext; +/// Where in the target file the new task goes +/// +/// Most placements resolve to a concrete line, but appending to a file whose +/// `## Tasks` section holds no tasks yet cannot: a parsed +/// [`TaskFile`] records task line numbers and nothing about section +/// boundaries, so the line is only knowable from the source text the emitter +/// reads. [`Self::EndOfTasksSection`] carries that intent through to the +/// emitter rather than guessing a number here. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum InsertAnchor { + /// A concrete 1-indexed source line; the task is inserted before it. + Line(usize), + + /// After the last content line of the `## Tasks` section. + /// + /// Used when the section has no tasks to append after. If the file has no + /// `## Tasks` heading at all, the emitter appends at the end of the file. + EndOfTasksSection, +} + +impl InsertAnchor { + /// The 1-indexed line this anchor names, if it names one + /// + /// Returns `None` for [`Self::EndOfTasksSection`], which only the emitter + /// can turn into a line. + #[must_use] + pub fn line(self) -> Option { + match self { + Self::Line(line_number) => Some(line_number), + Self::EndOfTasksSection => None, + } + } +} + /// Information about where to insert a task in a file /// /// Contains all the placement details needed by the emitter to write /// the new task to the correct location with proper formatting. #[derive(Debug, Clone, PartialEq, Eq)] pub struct PlacementInfo { - /// Line number to insert at (1-indexed) - /// - /// Special value 0 means insert at beginning of new file. - pub line_number: usize, + /// Where to insert the task + pub anchor: InsertAnchor, /// Order index among siblings (0-indexed) pub order_index: usize, @@ -54,7 +86,7 @@ impl PlacementResolver { /// # Examples /// /// ``` - /// use lash_core::creation::placement::{PlacementResolver, PlacementInfo}; + /// use lash_core::creation::placement::{InsertAnchor, PlacementResolver, PlacementInfo}; /// use lash_core::creation::validation::{TaskValidator, ValidationContext}; /// use lash_types::config::ConfigBuilder; /// use lash_types::creation::{TaskCreationRequest, FileTarget, ParentRef, InsertPosition}; @@ -82,7 +114,9 @@ impl PlacementResolver { /// /// // Then resolve placement /// let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); - /// assert_eq!(placement.line_number, 0); // New file + /// // The file has no tasks, so the line is resolved from the source text + /// // when the task is written. + /// assert_eq!(placement.anchor, InsertAnchor::EndOfTasksSection); /// assert_eq!(placement.order_index, 0); /// assert_eq!(placement.indent_level, 0); // Top-level task /// ``` @@ -103,33 +137,26 @@ impl PlacementResolver { /// /// Finds the last task at the same level and inserts after it. fn resolve_append(ctx: &ValidationContext) -> PlacementInfo { - // Handle empty file case - if ctx.resolved_file.tasks.is_empty() { - return PlacementInfo { - line_number: 0, // Signal for new file - order_index: 0, - indent_level: ctx.computed_depth as usize, - }; - } - let siblings = Self::get_siblings(&ctx.resolved_file, ctx.parent_task.as_ref()); let order_index = siblings.len(); - // Compute line number: after last sibling's subtree - let line_number = if let Some(last_sibling) = siblings.last() { - Self::find_end_of_task_subtree(&ctx.resolved_file, last_sibling) + 1 + let anchor = if let Some(last_sibling) = siblings.last() { + // After the last sibling's subtree + InsertAnchor::Line(Self::find_end_of_task_subtree(&ctx.resolved_file, last_sibling) + 1) } else if let Some(parent) = &ctx.parent_task { // No siblings, insert right after parent (accounting for annotations) let parent_line = Self::get_task_line(&ctx.resolved_file, parent); let annotation_lines = Self::count_annotation_lines(parent); - parent_line + annotation_lines + 1 + InsertAnchor::Line(parent_line + annotation_lines + 1) } else { - // No siblings and no parent - append at end of tasks section - Self::find_end_of_tasks_section(&ctx.resolved_file) + // Nothing to append after: the file has no tasks at this level and + // no parent to hang off. Only the source text says where the + // section ends, so defer to the emitter. + InsertAnchor::EndOfTasksSection }; PlacementInfo { - line_number, + anchor, order_index, indent_level: ctx.computed_depth as usize, } @@ -159,7 +186,7 @@ impl PlacementResolver { let line_number = Self::get_task_line(&ctx.resolved_file, target); Ok(PlacementInfo { - line_number, + anchor: InsertAnchor::Line(line_number), order_index: index, indent_level: ctx.computed_depth as usize, }) @@ -188,7 +215,7 @@ impl PlacementResolver { let line_number = Self::get_task_line(&ctx.resolved_file, task); Ok(PlacementInfo { - line_number, + anchor: InsertAnchor::Line(line_number), order_index, indent_level: ctx.computed_depth as usize, }) @@ -221,38 +248,12 @@ impl PlacementResolver { let line_number = Self::find_end_of_task_subtree(&ctx.resolved_file, task) + 1; Ok(PlacementInfo { - line_number, + anchor: InsertAnchor::Line(line_number), order_index, indent_level: ctx.computed_depth as usize, }) } - /// Find the end of the ## Tasks section (before next section or EOF) - /// - /// For now, we estimate based on the last task in the file. - /// In a future enhancement, this could parse the actual markdown structure. - fn find_end_of_tasks_section(file: &TaskFile) -> usize { - if file.tasks.is_empty() { - // Estimate: after file header (typically ~10 lines) - return 15; - } - - // Find the last top-level task and return line after its subtree - let top_level_tasks: Vec<_> = file - .tasks - .tasks() - .iter() - .filter(|t| t.parent_id.is_none()) - .collect(); - - if let Some(last_task) = top_level_tasks.last() { - Self::find_end_of_task_subtree(file, last_task) + 1 - } else { - // Fallback: estimate - 15 - } - } - /// Find the end of a task's subtree (last descendant line number) /// /// Returns the line number where the task and all its descendants end. @@ -443,7 +444,9 @@ mod tests { let ctx = validator.validate(&request, None).unwrap(); let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); - assert_eq!(placement.line_number, 0); // New file signal + // No tasks to append after and no `## Tasks` line numbers to work + // from, so the emitter resolves the line from the source text. + assert_eq!(placement.anchor, InsertAnchor::EndOfTasksSection); assert_eq!(placement.order_index, 0); assert_eq!(placement.indent_level, 0); // Top-level } @@ -469,7 +472,7 @@ mod tests { assert_eq!(placement.order_index, 2); // After 2 existing tasks assert_eq!(placement.indent_level, 0); // Top-level - assert!(placement.line_number > 0); + assert!(placement.anchor.line().unwrap() > 0); } #[test] @@ -508,7 +511,7 @@ mod tests { assert_eq!(placement.order_index, 1); // After 1 existing child assert_eq!(placement.indent_level, 1); // Depth 1 - assert!(placement.line_number > 0); + assert!(placement.anchor.line().unwrap() > 0); } #[test] @@ -965,7 +968,7 @@ mod tests { let request = TaskCreationRequestBuilder::new("Level 1-5").build(); let ctx = validator.validate(&request, Some(&file)).unwrap(); let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); - assert_eq!(placement.line_number, 38); + assert_eq!(placement.anchor.line(), Some(38)); assert_eq!(placement.order_index, 4); // Test: insert after Level 1-3 should go on line 35 (34 = last @@ -975,7 +978,7 @@ mod tests { .build(); let ctx = validator.validate(&request, Some(&file)).unwrap(); let placement = PlacementResolver::resolve(&ctx, &request).unwrap(); - assert_eq!(placement.line_number, 35); + assert_eq!(placement.anchor.line(), Some(35)); assert_eq!(placement.order_index, 3); } @@ -1009,7 +1012,7 @@ mod tests { // Checkbox on line 6, `@id:` annotation on line 7 -> insert at line 8, // not line 7 (which would land on top of the `@id:` line). - assert_eq!(placement.line_number, 8); + assert_eq!(placement.anchor.line(), Some(8)); assert_eq!(placement.order_index, 1); } @@ -1036,7 +1039,9 @@ mod tests { let ctx = validator.validate(&request, Some(file)).unwrap(); PlacementResolver::resolve(&ctx, &request) .unwrap() - .line_number + .anchor + .line() + .expect("appending after an existing task resolves to a line") } #[test] diff --git a/crates/lash-core/src/parser/header.rs b/crates/lash-core/src/parser/header.rs index 666f4a0..c21c9c1 100644 --- a/crates/lash-core/src/parser/header.rs +++ b/crates/lash-core/src/parser/header.rs @@ -348,6 +348,78 @@ pub fn content_has_tasks_section(content: &str) -> bool { false } +/// Line span of the body of the `## Tasks` section +/// +/// The range is 0-indexed and half-open: it starts at the first line after the +/// `## Tasks` heading and ends at the line holding the next H1 or H2 heading, +/// or at the end of the file. H3 and deeper headings sit inside the section +/// (files often group tasks under `### Subsection` headings) and do not close +/// it. Returns `None` if the file has no `## Tasks` heading. +/// +/// A parsed [`lash_types::TaskFile`] records task line numbers but no section +/// boundaries, so anything that needs the extent of the Tasks section (in +/// particular appending to a section with no tasks in it yet) has to go back to +/// the source text. Heading detection runs through pulldown-cmark, so a `##` +/// inside a fenced code block does not close the section. +/// +/// # Example +/// +/// ``` +/// use lash_core::parser::header::tasks_section_body; +/// +/// let content = "# T\n\n## Tasks\n\n- [ ] One\n\n## Notes\n\nprose\n"; +/// // Lines 3 through 5: the blank line, the task, and the blank line after it. +/// assert_eq!(tasks_section_body(content), Some(3..6)); +/// +/// // An empty section still has a body, it is just blank. +/// assert_eq!(tasks_section_body("# T\n\n## Tasks\n"), Some(3..3)); +/// +/// assert_eq!(tasks_section_body("# T\n\nno section here\n"), None); +/// ``` +#[must_use] +pub fn tasks_section_body(content: &str) -> Option> { + let mut heading_level: Option = None; + let mut heading_text = String::new(); + let mut body_start_byte: Option = None; + let mut body_end_byte = content.len(); + + for (event, range) in Parser::new(content).into_offset_iter() { + match event { + Event::Start(Tag::Heading { level, .. }) => { + if body_start_byte.is_some() && matches!(level, HeadingLevel::H1 | HeadingLevel::H2) + { + body_end_byte = range.start; + break; + } + heading_level = Some(level); + heading_text.clear(); + } + Event::Text(text) if heading_level.is_some() => heading_text.push_str(&text), + Event::End(TagEnd::Heading(level)) => { + if body_start_byte.is_none() + && level == HeadingLevel::H2 + && heading_text.trim().eq_ignore_ascii_case("tasks") + { + body_start_byte = Some(range.end); + } + heading_level = None; + } + _ => {} + } + } + + let body_start_byte = body_start_byte?; + Some(line_at_byte(content, body_start_byte)..line_at_byte(content, body_end_byte)) +} + +/// 0-indexed line holding the given byte offset +fn line_at_byte(content: &str, byte: usize) -> usize { + content[..byte.min(content.len())] + .bytes() + .filter(|b| *b == b'\n') + .count() +} + /// Find the ## Description section heading /// /// Returns the line number where "## Description" appears (0-indexed). diff --git a/lash.index.md b/lash.index.md index 29c1dc1..b2c0908 100644 --- a/lash.index.md +++ b/lash.index.md @@ -166,7 +166,7 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. - Regression test: emit a task with a two-line note, reparse the written file, assert both lines survive; plus a round-trip property test over notes with varying line counts and indentation -- [ ] `lash add` prepends above the H1 when the target file has no tasks yet, and the task is never indexed #cli #bug #data-loss +- [x] `lash add` prepends above the H1 when the target file has no tasks yet, and the task is never indexed #cli #bug #data-loss - Severe: the task is written to disk, invisible to Lash, and `lash lint` passes clean. Found 2026-08-08 while verifying the multi-line note fix - Repro: a well-formed file with a header and an empty `## Tasks` section @@ -189,6 +189,15 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. section, (b) an existing file with a `## Tasks` section followed by other sections, (c) a genuinely new file — assert the header survives at line 1 in (a) and (b), and that `lash index` finds the task in all three + - Fixed by replacing the magic 0 with `InsertAnchor`, which distinguishes a + concrete `Line(n)` from `EndOfTasksSection`. The emitter resolves the + latter against the source text, since a parsed `TaskFile` records task + line numbers but no section boundaries. New + `parser::header::tasks_section_body` returns the section's line span and + is markdown-aware, so a `##` inside a code fence does not close it. The + 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 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