fix: best-effort fallback when slash line is invalid grammar#217
Merged
nettapper merged 3 commits intoMay 26, 2026
Merged
Conversation
ParseTask currently aborts the entire parse when a line starts with `/` but isn't a valid slash command (e.g. `//`, lone `/`, `/=foo`). These should fall through to text, preserving both the malformed line and any surrounding content. In-fence slashes are already handled by the markdown-aware path; only the bare-text case is broken.
A single malformed slash line (e.g. `//`, lone `/`, `/=foo`) outside a fenced code block caused ParseTask to abort the entire task, silently dropping the user prompt and frontmatter and corrupting downstream runs. On parser failure, parseGrammar now re-attempts line-by-line. Lines that still fail are emitted as raw text with a WARN logged at slog.Default. The whole-segment fast path is unchanged for valid input.
Adds *WithLogger variants for the parse entry points so callers can route taskparser's best-effort fallback WARN logs through their own slog.Logger instead of slog.Default(): - taskparser.NewExtension(logger) (Extension stays as NewExtension(nil)) - taskparser.ParseTaskWithLogger(text, logger) - markdown.ParseMarkdownFileWithLogger(path, fm, logger) The original ParseTask/ParseMarkdownFile/Extension entry points are preserved as nil-logger wrappers, so external callers are unaffected. A nil logger resolves to slog.Default() at log time (via loggerOrDefault) rather than being captured at construction, so slog.SetDefault changes made after package load still take effect. A new test covers this. All five ParseMarkdownFile callsites and the ParseTask callsite in pkg/codingcontext/context.go now pass cc.logger.
swedishborgie
approved these changes
May 26, 2026
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
ParseTaskaborted the entire task with a hard error when a line outside afenced code block started with
/but wasn't a valid slash command (e.g.//, lone/,/=foo). One bad line in an appended prompt was enoughto discard the entire prompt, the frontmatter, and every sibling task in the
directory.
In-fence slashes were already handled by the markdown-aware splitter; only
the bare-text case was broken.
Also threads a
*slog.Loggerthrough the parse entry points so callers canroute the new WARN logs through their own logger instead of
slog.Default().What changed
pkg/codingcontext/taskparser/markdownparser.go—parseGrammarnowfalls back to a line-by-line parse on failure. Lines that still don't parse
are emitted as raw text and a WARN is logged with the offending line and
the underlying grammar error. The whole-segment fast path is unchanged for
valid input.
API additions (backward compatible):
taskparser.NewExtension(logger *slog.Logger) goldmark.Extender— theexisting
Extensionglobal is nowNewExtension(nil).taskparser.ParseTaskWithLogger(text, logger)—ParseTaskis anil-logger wrapper.
markdown.ParseMarkdownFileWithLogger(path, fm, logger)—ParseMarkdownFileis a nil-logger wrapper.Nil loggers resolve to
slog.Default()at log time (not capture time), soslog.SetDefaultchanges made after package load still take effect.pkg/codingcontext/context.go— all fiveParseMarkdownFilecallsites and the one
ParseTaskcall site now passcc.logger, so parserWARNs route through whatever logger was injected via
codingcontext.WithLogger.pkg/codingcontext/taskparser/taskparser_test.go— new coverage:TestParseTask_BestEffortMalformedSlash— three subcases (//, lone/,/=foo) outside a code block, each asserting both the malformedline and the surrounding text survive (guards against a fix that
salvages only the bad line).
TestParseTaskWithLogger_RoutesWarnToInjectedLogger— captures stderrfrom an injected logger and verifies the WARN lands there.
TestParseTaskWithLogger_NilLoggerResolvesLazyDefault— usesslog.SetDefaultafter construction to confirm nil-logger callers getthe current default, not a snapshot.
Out of scope
grammar to treat any unrecognized
/-prefixed line as text directly) wouldbe cleaner but riskier.