Skip to content

fix(bash): let the output preview use the whole line budget - #858

Open
ANIRUDDHA ADAK (aniruddhaadak80) wants to merge 1 commit into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/bash-output-line-budget
Open

ANIRUDDHA ADAK (aniruddhaadak80) wants to merge 1 commit into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/bash-output-line-budget

Conversation

@aniruddhaadak80

Copy link
Copy Markdown
Contributor

What does this PR do, and why?

The bash output preview was allowed one line less than its budget, so output of exactly the maximum dropped its last line and came back reported as truncated.

The two bounds in BashOutput.Capture.accept() disagreed with each other:

const fits =
  this.previewBytes + size <= this.options.maxBytes &&   // bytes: the budget is reachable
  this.previewLines + newlines < this.options.maxLines    // lines:  one short of it

head() used the matching strict comparison, so the preview consistently stopped at maxLines - 1. With maxLines: 5:

emitted=4  previewKept=4  truncated=false
emitted=5  previewKept=4  truncated=true    <- 5 lines in, 4 kept
emitted=6  previewKept=4  truncated=true    <- should keep 5 and drop 1

bash.ts passes Truncate.MAX_LINES (2000) and the tool description advertises that number, so a command printing exactly 2000 lines — a build log, a test run — loses its last line, is reported as truncated with "1 lines truncated", and has its whole output written to a second file the user then has to open for nothing. Every count is off by one as well: 2001 lines reports two removed rather than one.

A budget is the most that fits, which is what the byte bound beside it already did and what truncation.ts:107 already does for the same comparison. The line bound becomes <= and head()'s break becomes > lineBudget to match.

One existing expectation changes, deliberately

bash-output.test.ts had recorded the off-by-one instead of catching it:

-    expect(summary.removed).toEqual({ count: lines - 99, unit: "lines" })
-    expect(summary.preview.split("\n")).toHaveLength(100)
+    // The preview holds the full 100-line budget, not 99: the line test was a
+    // strict `<` while the byte test beside it was `<=`, so the old expectation
+    // below was recording the off-by-one this suite now pins.
+    expect(summary.removed).toEqual({ count: lines - 100, unit: "lines" })
+    expect(summary.preview.split("\n")).toHaveLength(101)

That test is about streaming the full output to the owned file, and both of its numbers were boundary artifacts of the bug. Everything else it asserts — truncated, lines, the redaction, the single sink open, the saved file's length and its first and last lines — is unchanged and still passes.

Linked issue

Fixes #857

How did you verify it?

Both tests fail against unmodified main and pass with the change:

$ bun test --timeout 15000 ./test/tool/bash-output.test.ts     # on main

error: expect(received).toMatchObject(expected)
(fail) BashOutput.Capture > output of exactly the line limit keeps every line [1844.33ms]
error: expect(received).toEqual(expected)
- Expected  - 1
+ Received  + 1
(fail) BashOutput.Capture > streams the full redacted output to the owned file once the preview overflows [1418.85ms]

 10 pass
 2 fail
$ bun test --timeout 15000 ./test/tool/bash-output.test.ts     # with the change

 12 pass
 0 fail

The new case pins both sides of the boundary in one test — exactly the limit keeps every line and reports removed: { count: 0 }, and one line over keeps 5 and reports removed: { count: 1 } — so a fix that simply raised the limit could not pass it.

The suite is self-contained (no Instance harness), so all 12 results are deterministic here.

Formatting checked with the repository's own Prettier config (semi: false, printWidth: 120) on LF-normalized copies, since this Windows checkout's core.autocrlf=true otherwise makes Prettier flag every file repo-wide.

Checklist

  • bun run check is green (format, typecheck, backend + frontend/ui + SDK tests)
  • bun run --cwd frontend/workspace build succeeds if I touched frontend/workspace or frontend/ui
  • ./tooling/repo/generate.ts was run and the tooling/sdk output committed if I changed backend/cli/src/server
  • CHANGELOG.md has an Unreleased entry if the change is user-visible
  • The matching docs page under frontend/docs/src/content/openscience/ is updated if behavior changed
  • Screenshots or a short video are attached for UI changes
  • No version bumps (package.json versions and tags are written by the release workflow)
  • install and frontend/landing/public/install are still byte-identical if I touched either

@vercel

vercel Bot commented Sep 30, 2026

Copy link
Copy Markdown

ANIRUDDHA ADAK (@aniruddhaadak80) is attempting to deploy a commit to the InkVell Team on Vercel.

A member of the Team first needs to authorize it.

The line test was a strict < while the byte test beside it was <=, so the effective limit was one line shorter than the advertised one. Output of exactly maxLines lines was treated as an overflow: the last line was dropped, truncated came back true, and the entire stream was copied to a second file. head() moved with it so the two agree. truncation.ts already uses <= for the same comparison.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bash output drops a line when the output is exactly the line limit, and reports it as truncated

1 participant