Skip to content

pretty cli output - #1090

Merged
Olmo Maldonado (ibolmo) merged 2 commits into
mainfrom
pretty-cli-output
Dec 1, 2025
Merged

pretty cli output#1090
Olmo Maldonado (ibolmo) merged 2 commits into
mainfrom
pretty-cli-output

Conversation

@edenh

Copy link
Copy Markdown
Contributor
Screenshot 2025-11-19 at 10 59 10 PM

@clutchski

Copy link
Copy Markdown
Collaborator

looks great.

Comment thread js/package.json
"simple-git": "^3.21.0",
"slugify": "^1.6.6",
"source-map": "^0.7.4",
"terminal-link": "^5.0.0",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not very knowledgable about js/ts/node deps, but if these have possibility of conflicting with customer's dependencies, we might not want to include them and either (a) reimplement the functionality or (b) copy their source into our tree, depending on how complicated they are. I imagine terminal-link isn't that complicated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think these are pretty harmless. they are simple CLI display utilities like the existing chalk​ and cli-progress​ packages

@cpinn Caitlin Pinn (cpinn) Dec 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this new dependency is causing an issue for cloudflare workers when attempting to use the mjs (esm modules) file of braintrust. This is because terminal link has a hard dependency on a 10.0 version of supports-color (brought in by supports-hyperlink) that doesn't export a necessary method in their browser build.

I had to temporarily remove the terminal-link dependency here: #1137. It doesn't remove any of the other nice updates from this PR and we will look at bringing the code back probably by porting over some of the code.

We have also made a test to discover this type of issue sooner.

Comment thread js/scripts/test-output.ts
import type { ExperimentSummary } from "../src/logger";
import { BarProgressReporter } from "../src/progress";

function createSampleSummary(options: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice if we could move it to an actual test some where some how. i would forget to check the output like this.

i suppose we don't expect this to change much, but would still be nice.

@ibolmo
Olmo Maldonado (ibolmo) merged commit 6cb3c40 into main Dec 1, 2025
34 checks passed
@ibolmo
Olmo Maldonado (ibolmo) deleted the pretty-cli-output branch December 1, 2025 23:52
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.

5 participants