Reliability fixes#41
Merged
Merged
Conversation
embeds-update inserted each result by matching "\n" + line + "\n", but the first line of a script has no leading newline, so its replacement silently no-op'd even though the output was computed and zipped. Seed the fold with a prepended newline to anchor the first line like any interior line, then strip that one newline back off. Add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`module-commands-code-to-record` used `scan --noinit null` to forward-fill the last-seen command name down the lines of a module. Nushell's std `scan` now delegates to `generate`, which treats a `null` seed as "no initial value" and errors out — so the command (and its two unit tests) broke entirely. dotnu can't patch std, so stop seeding `scan` with null: forward-fill the last non-null name manually with a `reduce` instead. `[] | last` returns null on the empty accumulator, so no optional handling is needed. Drops the now-dead `use std/iter scan` import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`find-capture-points` (the detector) matched `\|\s?print \$in *$` while `execute-and-parse-results` (the rewriter) matched `\| *print +\$in *`. The two disagreed on real inputs — e.g. two spaces after `|`, which the detector missed but the rewriter still rewrote, and `foo | print $in | bar`, which the rewriter would rewrite but the detector ignored. Since `embeds-update` zips the detector's points against the rewriter's outputs, any disagreement silently embeds output against the wrong line. Extract one module-level `const capture_point` used by both. Anchor it to end-of-line so a mid-pipeline `| print $in` is consistently treated as not a capture point by both sides. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`run-snapshot-test` decided pass/changed with `git diff --quiet`, which only considers tracked files. A brand-new snapshot file is untracked, so its first run produced no diff and was reported 'passed' — a false green that hides an unreviewed baseline. Check `git ls-files --error-unmatch` first: an untracked output is 'changed', so it surfaces in the summary and `--update` stages it like any other change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The embeds-update / embeds-remove integration tests ran on a fixture containing `random int` and `ls | sort-by modified` — random and filesystem-dependent — so the snapshot could never match and was flagged 'changed' on every run. Replace those two capture points with deterministic equivalents (`40 + 2` and a literal table) and keep the multi-line str-replace pipeline. Coverage is unchanged: scalar capture, multi-line table capture, and multi-physical-line pipeline are all still exercised, and the subprocess `table -e` is width-stable. The input `# =>` values are intentionally stale/wrong (`0`, `stale`, `WRONG`). Why: `embeds-update` strips every `# =>` then re-executes and re-embeds, so a fresh value can only appear if the round-trip actually ran. Wrong inputs mean a silent no-op or a skipped capture point would leave the wrong value and fail the snapshot — keeping the end-to-end test honest under determinism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The coverage snapshot drifted between nushell versions purely because `to yaml` changed how it indents nested scalar lists (`- x` vs ` - x`); the data was identical. Serialize the coverage record as nuon instead, so the snapshot compares the parsed structure rather than yaml's incidental formatting. Renames the fixture coverage-untested.yaml -> .nuon (kept in output-yaml/). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
maxim-uvarov
pushed a commit
that referenced
this pull request
Jul 9, 2026
Audited against current source: the 'Operation interrupted' hang on dotnu dependencies for numd/commands.nu no longer reproduces. The exact original file (722 lines) now returns 178 rows in well under the 60s timeout; reliability fixes landed in PR #41 (f7dc262) and the command now sorts output deterministically. Other 5 todos verified still pending, kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Running
nu toolkit.nu teston a clean checkout was not green: 2 unit tests failed outright and 2 snapshot tests drifted on every run. This branch fixes the root causes — one real breakage, one latent bug, and three test-honesty issues — and gets the suite to 74 passed, 0 failed, 0 changed.Bugs
module-commands-code-to-recordbroke on nushell 0.113 (47422c4). It forward-filled command names withscan --noinit null; stdscannow delegates togenerate, which rejects anullseed as "no initial value", so the command and its two unit tests errored out. Since dotnu can't patch std, the fix forward-fills with areduceinstead and drops the now-deaduse std/iter scanimport.Capture-point regex had two disagreeing definitions (
5c884ee). The detector (find-capture-points) matched\|\s?print \$in *$while the rewriter (execute-and-parse-results) matched\| *print +\$in *. They diverged on real inputs (two spaces after|, mid-pipeline| print $in | bar), and sinceembeds-updatezips the two together, a disagreement silently embeds output against the wrong line. Replaced with one module-levelconst capture_pointused by both, anchored to end-of-line.Test honesty
New snapshots reported false
passed(f1eac60).run-snapshot-testusedgit diff --quiet, which ignores untracked files, so a brand-new snapshot's first run never compared against anything. Now an untracked output ischangedso it surfaces and--updatestages it.embeds-update snapshot was nondeterministic (
850b4e6). Its fixture containedrandom intandls | sort-by modified— random and filesystem-dependent — so it could never match. Replaced those with deterministic equivalents (40 + 2, a literal table) while keeping full round-trip coverage (scalar capture, multi-line table capture, multi-line pipeline). The input# =>values are intentionally wrong, so a silent no-op or skipped capture point leaves the wrong value and fails the snapshot — keeping the end-to-end test honest under determinism.coverage snapshot drifted on serializer formatting (
b429bdb). It comparedto yamltext, whose list indentation changed between nushell versions on identical data. Switched to nuon so the snapshot compares parsed structure, not yaml's incidental formatting.