Quiet-by-default logging with --json for agents#18
Conversation
Rewrites the internal logger as a dual-form {human, json} API so each
call site spells out both shapes explicitly. Default human mode is
terse and prefix-free on stderr; LEADTYPE_LOG_FORMAT=json (or --json
on CLI commands) emits one structured event per line for agents and
CI; LEADTYPE_VERBOSE=1 / --verbose firehoses per-file events.
- Collapse per-file convert chatter into one batch summary with timing
- Migrate stray process.stderr.write callers in search/node.ts and
remark/plugins/include.remark.ts to the new logger
- Move success summaries from stdout to stderr so pipelines stay clean
- Couple --json to log format in generate and lint CLIs; add --verbose
- Silence rollup build output and bun run preambles so the example
app's bun dev flow is ~4 lines instead of the prior 30+
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces a string-based log helper with a structured, configurable logger (human/json formats, verbosity, and stream targets) and updates CLI, generate/lint commands, MDX conversion, remark include plugin, search/vercel modules, scripts, package-scripts (silenced), and tests to emit structured log events. ChangesLogging Infrastructure and CLI Integration
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Module
participant logger
participant stderr
User->>CLI: run leadtype generate -v
CLI->>Module: invoke generate
Module->>logger: logger.info / logger.error
logger->>stderr: emit human or JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdd8cb1c71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (args.format === "json") { | ||
| setLogFormat("json"); | ||
| } | ||
| if (args.verbose) { | ||
| setVerbose(true); |
There was a problem hiding this comment.
Because setLogFormat("json") and setVerbose(true) mutate module-global logger state and are only called when the flag is present, an in-process caller that reuses the exported runCli after a generate --format json or --verbose run will keep emitting JSON/debug logs on later default invocations. This affects test harnesses or agent integrations that call the CLI API multiple times in one process; set the logger format and verbosity from the parsed args on every invocation or scope the settings to the command run.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/cli.test.ts`:
- Around line 114-116: The test currently asserts exit code, stderr and that a
file exists but doesn't lock down stdout; update the test in cli.test.ts to also
assert that capture.stdout is empty for the text-mode path by adding an
assertion using the existing capture object (e.g.,
expect(capture.stdout).toBe("") or equivalent) so stdout remains clean and no
human logs leak into pipeline output.
In `@packages/leadtype/src/cli/generate.ts`:
- Around line 361-366: The current code only calls setLogFormat and setVerbose
when flags are present causing runtime state to leak between invocations; make
logger configuration deterministic by always calling setLogFormat and setVerbose
based on args: call setLogFormat(args.format === "json" ? "json" : "default"/*or
explicit non-json value your logger expects*/) and call
setVerbose(Boolean(args.verbose)) so every command invocation resets logging
state regardless of whether flags were provided; update the logic around
args.format and args.verbose in generate.ts to use these unconditional calls
(referencing args.format, args.verbose, setLogFormat, setVerbose).
In `@packages/leadtype/src/convert/convert.ts`:
- Around line 465-470: The current failure log misleadingly hints that
LEADTYPE_VERBOSE=1 will print a stack even though this code path never emits
error.stack; update logger.error in convert.ts so that if reason is an Error you
include reason.stack (e.g. add stack to human.message or human.hint and to
json.fields) and otherwise change the hint text to not promise a stack (e.g.
"run with LEADTYPE_VERBOSE=1 for more verbose logs"); specifically modify the
logger.error call that references mdxFilePath and reason to conditionally
include reason.stack when available and adjust the hint accordingly.
In `@packages/leadtype/src/internal/logger.ts`:
- Around line 33-40: The JSON log output currently spreads call.json.fields
after reserved keys (ts, level, event) so those reserved keys can be overwritten
or removed; change the serialization in logger.ts so that when building the
object for JSON output you first merge sanitized fields and then set reserved
keys (or filter call.json.fields to remove keys "ts", "level", and "event" and
drop undefined values) before stringifying; locate the block that writes
`${JSON.stringify({ ts: ..., level, event: call.json.event, ...call.json.fields
})}` and update it to exclude/ignore any fields named "ts", "level", or "event"
(and skip undefined values) so reserved schema keys cannot be overwritten.
In `@packages/leadtype/src/lint/cli.ts`:
- Around line 137-142: The CLI currently only toggles json/verbose when flags
are present, so previous runs can leak logging state; in the lint CLI entry
(packages/leadtype/src/lint/cli.ts) explicitly reset logger state at the start
of each run by calling setLogFormat("text" or the default format) and
setVerbose(false) before applying the args checks, then apply the existing if
(args.format === "json") setLogFormat("json") and if (args.verbose)
setVerbose(true); this ensures setLogFormat and setVerbose are always
deterministically initialized per invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ecf1040-f74b-4db8-aa50-d262965e8756
📒 Files selected for processing (13)
apps/example/package.jsonapps/example/scripts/mdx-convert.tspackages/leadtype/package.jsonpackages/leadtype/scripts/generate-docs.tspackages/leadtype/src/cli.test.tspackages/leadtype/src/cli.tspackages/leadtype/src/cli/generate.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/lint/cli.tspackages/leadtype/src/remark/plugins/include.remark.tspackages/leadtype/src/search/node.tspackages/leadtype/src/search/vercel.ts
💤 Files with no reviewable changes (1)
- apps/example/scripts/mdx-convert.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/search/vercel.tspackages/leadtype/scripts/generate-docs.tspackages/leadtype/src/lint/cli.tspackages/leadtype/src/cli.tspackages/leadtype/src/remark/plugins/include.remark.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/search/node.tspackages/leadtype/src/cli/generate.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/cli.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/search/vercel.tspackages/leadtype/scripts/generate-docs.tspackages/leadtype/src/lint/cli.tspackages/leadtype/src/cli.tspackages/leadtype/src/remark/plugins/include.remark.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/search/node.tspackages/leadtype/src/cli/generate.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/cli.test.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/cli.test.ts
🔍 Remote MCP Context7
Tool Library Documentation Retrieved
Bun --silent Flag
The PR's use of bun run --silent in package.json scripts aligns with Bun's documented behavior. The --silent flag suppresses the command execution preamble, preventing Bun from printing "Running '...'..." messages to the console. This can be set via CLI flags (bun run --silent pipeline:convert) or in bunfig.toml configuration. This is the standard approach for reducing noise in build pipelines and development workflows.
Rollup --silent Flag
Similarly, the PR's addition of --silent to Rollup invocations is a documented CLI option that suppresses warnings printed to the console. Importantly, Rollup's --silent flag does not suppress errors (which must be handled via the logLevel option); it only suppresses warning-level logs. Any onLog or onwarn handlers defined in the config or plugins are still executed even with --silent enabled. This is relevant for the PR since it's combining Rollup's --silent with a structured logging migration—the logging handlers will still fire.
Remark Plugin Architecture
The PR modifies packages/leadtype/src/remark/plugins/include.remark.ts to emit structured logger calls instead of direct process.stderr.write statements. This aligns with Remark's plugin pattern: plugins receive the markdown AST (mdast) and return a transform function that visits and modifies nodes. The standard approach uses unist-util-visit to traverse the tree. The PR's migration to structured logging within the plugin is compatible with this architecture—plugins can emit events/logs without breaking the tree transformation flow.
PR-Specific Insights
-
Output Silencing Strategy: The PR uses both tool-level flags (
bun run --silent,rollup --silent) and application-level structured logging to achieve quiet-by-default behavior. This is a sound dual-layer approach. -
Stderr as Default Output Stream: Moving success summaries from stdout to stderr (e.g., in
convert.ts,cli.ts,generate.ts) is appropriate for keeping generated artifacts on stdout clean for pipeline consumption while relegating logging and diagnostics to stderr. -
Structured Logging Compatibility: The new logger API with
setLogFormat("json")andsetVerbose(true)is compatible with how the underlying libraries (Rollup, Bun, Remark) handle their own logging—they all support structured event emission or suppression independently.
🔇 Additional comments (7)
apps/example/package.json (1)
7-8: LGTM! Clean reduction of pipeline noise.The addition of
--silentto thebun run pipeline:convertsteps in bothdevandbuildscripts appropriately suppresses Bun's command execution preamble while preserving error output. This aligns with the PR's goal of reducing the example app's output from ~30+ lines to ~4 lines and keeps the approach consistent across both development and production build flows.packages/leadtype/src/search/vercel.ts (1)
110-117: Structured provider-error logging looks solid.Nice migration to
logger.errorwith a stable event name and explicitreasonfield.packages/leadtype/src/search/node.ts (1)
173-181: Large-index warning refactor is clean.The new human+JSON warning payload is clear and keeps diagnostics machine-friendly.
Also applies to: 183-221
packages/leadtype/src/remark/plugins/include.remark.ts (1)
368-378: Good move to structured include warnings.These warnings now flow through the shared logger with useful event metadata.
Also applies to: 438-446
packages/leadtype/src/cli.ts (1)
40-40: CLI entrypoint logging integration looks good.
setLogStreams(io)plus structuredcli.fatalemission is a solid migration.Also applies to: 77-80
packages/leadtype/scripts/generate-docs.ts (1)
38-44: Structured logger migration here looks solid.Both error and success paths now emit consistent
{ human, json }events with useful fields, which matches the stderr-first/pipeline-safe logging direction.Also applies to: 60-68
packages/leadtype/package.json (1)
72-72: Build-script quieting is aligned with the PR goal.Using
--silentin both commands keeps build output focused while preserving the existing build flow.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/cli/generate.ts`:
- Around line 361-363: After calling setLogFormat(...) and setVerbose(...) bind
the logger output to the command-local io.stderr so logs go to the provided io
rather than global process.stderr; update the logging configuration (e.g., add a
call like setLogStreams/ioStreamSetter or equivalent) immediately after
setVerbose using the local io.stderr reference so subsequent logger calls (those
around the setLogFormat/setVerbose block and later lines 370/459/469) write to
io.stderr.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3bebba33-39a0-4d90-911c-2661a52bf805
📒 Files selected for processing (5)
packages/leadtype/src/cli.test.tspackages/leadtype/src/cli/generate.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/lint/cli.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/cli.test.tspackages/leadtype/src/lint/cli.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/cli/generate.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/cli.test.tspackages/leadtype/src/lint/cli.tspackages/leadtype/src/internal/logger.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/cli/generate.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/cli.test.ts
🔍 Remote MCP Context7
Summary of relevant facts for reviewing this PR
-
Bun: bun run --silent (or bun --silent run / run.silent = true in bunfig.toml) suppresses Bun’s “Running '…'…” preamble and prevents printing the command itself; it does not affect the command’s own stdout/stderr output. This aligns with the PR’s use of bun run --silent to reduce Bun preamble noise.
-
Rollup: rollup --silent prevents printing warnings to the console but does not suppress errors. Plugins or config-level onLog/onwarn handlers are still invoked when --silent is used; to fully suppress handler activity you must also change logLevel (e.g., --logLevel silent) or adjust onLog/onwarn. Thus adding --silent reduces Rollup warning chatter but will not hide errors or prevent plugin onLog hooks from running.
Tools/sources used
- Context7 — Context7_resolve-library-id (resolved Bun and Rollup library IDs)
- Context7 — Context7_query-docs (queried Bun and Rollup docs for --silent behavior)
🔇 Additional comments (4)
packages/leadtype/src/internal/logger.ts (1)
37-63: Reserved-field sanitization in JSON mode looks solid.Line 43 correctly drops
undefinedand reserved keys before emission, so schema keys remain stable in Line 57-62.packages/leadtype/src/lint/cli.ts (1)
137-144: Deterministic logger initialization is correctly implemented.Line 137-144 now resets and reapplies log mode/verbosity per run, which avoids cross-invocation leakage.
packages/leadtype/src/convert/convert.ts (1)
464-477: Failure logging is materially improved.Line 464-477 now preserves both message and stack (when available), improving debuggability in both human and JSON modes.
packages/leadtype/src/cli.test.ts (1)
114-117: Good test hardening for stream separation.Line 115-116 correctly locks in clean stdout and verifies the human completion log on stderr.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/leadtype/src/cli/generate.ts (1)
374-389:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRoute text-mode failures through
logger.errortoo.The non-JSON branches still write manual prefixed stderr strings, which bypasses the centralized human logger path and creates inconsistent output behavior.
Proposed minimal diff
if (!existsSync(docsDir)) { - if (args.format === "json") { - logger.error({ - human: { message: `docs directory not found at ${docsDir}` }, - json: { - event: "generate.docs_not_found", - fields: { error: "docs directory not found", path: docsDir }, - }, - }); - } else { - io.stderr.write( - `leadtype generate: docs directory not found at ${docsDir}\n` - ); - } + logger.error({ + human: { message: `docs directory not found at ${docsDir}` }, + json: { + event: "generate.docs_not_found", + fields: { error: "docs directory not found", path: docsDir }, + }, + }); return 1; } @@ } catch (error) { const message = error instanceof Error ? error.message : String(error); - if (args.format === "json") { - logger.error({ - human: { message }, - json: { - event: "generate.fail", - fields: { - error: message, - filters: { - exclude: args.exclude, - include: args.include, - }, - }, - }, - }); - } else { - io.stderr.write(`leadtype generate: ${message}\n`); - } + logger.error({ + human: { message }, + json: { + event: "generate.fail", + fields: { + error: message, + filters: { + exclude: args.exclude, + include: args.include, + }, + }, + }, + }); return 1; } finally {Also applies to: 472-490
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/leadtype/src/cli/generate.ts` around lines 374 - 389, The code currently writes human-readable error strings directly to stderr (using io.stderr.write) when args.format !== "json" (e.g., the existsSync(docsDir) check), bypassing the centralized logger; replace those io.stderr.write calls with logger.error(...) calls that use the human field (e.g., logger.error({ human: { message: `leadtype generate: docs directory not found at ${docsDir}` }, json: { event: "generate.docs_not_found", fields: { error: "docs directory not found", path: docsDir } } })) so both JSON and text modes go through the same logging path; apply the same change to the other occurrence referenced around the check at the later block (the code around lines handling the same error at 472-490) and keep the existing json payload intact for args.format === "json".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/leadtype/src/cli/generate.ts`:
- Around line 374-389: The code currently writes human-readable error strings
directly to stderr (using io.stderr.write) when args.format !== "json" (e.g.,
the existsSync(docsDir) check), bypassing the centralized logger; replace those
io.stderr.write calls with logger.error(...) calls that use the human field
(e.g., logger.error({ human: { message: `leadtype generate: docs directory not
found at ${docsDir}` }, json: { event: "generate.docs_not_found", fields: {
error: "docs directory not found", path: docsDir } } })) so both JSON and text
modes go through the same logging path; apply the same change to the other
occurrence referenced around the check at the later block (the code around lines
handling the same error at 472-490) and keep the existing json payload intact
for args.format === "json".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd140429-60c8-4f86-bf1a-f12fafd15a73
📒 Files selected for processing (1)
packages/leadtype/src/cli/generate.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/cli/generate.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/cli/generate.ts
🔍 Remote MCP Context7
Summary of additional facts relevant to this PR review
-
bun run --silent suppresses the “Running ''...” preamble (equivalent CLI forms: bun --silent run, bun --silent , bun run --silent). It prevents bun from printing the command being run but does not affect the command’s own stdout/stderr.
-
rollup --silent prevents printing warnings to the console but does not stop onLog/onwarn handlers or plugin onLog hooks from being invoked. To more fully suppress logging from Rollup (including handlers), use the logLevel option or pass --logLevel silent. Errors are still printed by the CLI.
Tools/sources used
- Context7: resolve-library-id + query-docs for Bun and Rollup documentation (see citations above).
🔇 Additional comments (1)
packages/leadtype/src/cli/generate.ts (1)
366-369: Good logger bootstrap for this command.This is now deterministic per invocation and correctly binds logs to
io.stderr.
…oisy-logs # Conflicts: # apps/example/package.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/search/node.ts`:
- Around line 177-185: The function formatBytes uses raw numeric literals (1024
and 1024 * 1024); extract these into named constants (e.g., BYTES_PER_KB = 1024
and BYTES_PER_MB = BYTES_PER_KB * 1024) near the top of the module and replace
the literals inside formatBytes with those constants so the comparisons and
divisions use BYTES_PER_MB and BYTES_PER_KB; update the string formatting logic
to use the constants for readability and consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f03b75e-57b8-4a47-a11f-d7d11557e578
📒 Files selected for processing (5)
apps/example/package.jsonpackages/leadtype/package.jsonpackages/leadtype/src/cli.test.tspackages/leadtype/src/cli/generate.tspackages/leadtype/src/search/node.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/search/node.tspackages/leadtype/src/cli.test.tspackages/leadtype/src/cli/generate.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/search/node.tspackages/leadtype/src/cli.test.tspackages/leadtype/src/cli/generate.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/cli.test.ts
🔍 Remote MCP Context7
Summary of additional facts relevant to reviewing this PR
-
Bun: "bun run --silent" / "bun --silent run" suppresses Bun's "Running ''..." preamble (the command-printing line) but does not alter or suppress the invoked command's own stdout/stderr. It is equivalent to setting run.silent = true in bunfig.toml.
-
Rollup: the CLI flag --silent prevents Rollup from printing warnings to the console but does not stop onLog/onwarn handlers or plugin onLog hooks from being invoked. To more fully silence Rollup (including handlers), use the logLevel option or pass --logLevel silent; errors are still printed.
Tooling lookups performed
- Resolved Context7 library IDs for Bun and Rollup before querying docs.,
🔇 Additional comments (8)
packages/leadtype/package.json (1)
76-76: Build script change is aligned with quiet-by-default goals.Using
--silenthere reduces wrapper/tool noise while keeping the build pipeline intact (rollupthen docs generation). This is consistent with the PR’s logging objective.apps/example/package.json (1)
7-8: Dev/build script updates look good.Switching
pipeline:buildtobun run --silentis a clean noise reduction change and keeps the existing workflow unchanged.packages/leadtype/src/search/node.ts (2)
5-5: Logger import change is aligned with the new logging architecture.Using the shared logger here keeps stderr handling centralized and consistent.
188-224: Breach aggregation and structured warning payload look solid.The early return plus single
logger.warnemission keeps logs concise while preserving machine-readable threshold data.packages/leadtype/src/cli/generate.ts (3)
47-47: Verbose flag wiring is clean and consistent.
GenerateArgs, defaults, parser handling, and help text all align for-v/--verbose.Also applies to: 116-117, 143-144, 178-180
372-375: Deterministic per-invocation logger setup looks correct.Configuring format, verbosity, and command-local
stderrup front is the right shape for command-scoped logging.
382-388: Structured event logging paths are well implemented.The
generate.docs_not_found,generate.done, andgenerate.failevents are explicit and keep machine payload output separated from operational logs.Also applies to: 481-487, 491-503
packages/leadtype/src/cli.test.ts (1)
115-116: Great test hardening for stdout/stderr routing.This correctly locks in the “quiet stdout” contract while still validating user-facing completion feedback on stderr.
| function formatBytes(bytes: number): string { | ||
| if (bytes >= 1024 * 1024) { | ||
| return `${(bytes / (1024 * 1024)).toFixed(1)} MB`; | ||
| } | ||
| if (bytes >= 1024) { | ||
| return `${(bytes / 1024).toFixed(1)} KB`; | ||
| } | ||
| return `${bytes} B`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract byte unit magic numbers into named constants.
Line 178 and Line 181 use raw unit values (1024, 1024 * 1024). Pull them into named constants for readability and consistency.
♻️ Proposed refactor
const WARN_INDEX_BYTES = 5 * 1024 * 1024;
const WARN_TOTAL_BYTES = 10 * 1024 * 1024;
const WARN_CHUNK_COUNT = 10_000;
+const BYTES_PER_KB = 1024;
+const BYTES_PER_MB = BYTES_PER_KB * 1024;
const WINDOWS_PATH_PATTERN = /\\/g;
@@
function formatBytes(bytes: number): string {
- if (bytes >= 1024 * 1024) {
- return `${(bytes / (1024 * 1024)).toFixed(1)} MB`;
+ if (bytes >= BYTES_PER_MB) {
+ return `${(bytes / BYTES_PER_MB).toFixed(1)} MB`;
}
- if (bytes >= 1024) {
- return `${(bytes / 1024).toFixed(1)} KB`;
+ if (bytes >= BYTES_PER_KB) {
+ return `${(bytes / BYTES_PER_KB).toFixed(1)} KB`;
}
return `${bytes} B`;
}As per coding guidelines, Use meaningful variable names instead of magic numbers - extract constants with descriptive names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/src/search/node.ts` around lines 177 - 185, The function
formatBytes uses raw numeric literals (1024 and 1024 * 1024); extract these into
named constants (e.g., BYTES_PER_KB = 1024 and BYTES_PER_MB = BYTES_PER_KB *
1024) near the top of the module and replace the literals inside formatBytes
with those constants so the comparisons and divisions use BYTES_PER_MB and
BYTES_PER_KB; update the string formatting logic to use the constants for
readability and consistency.
1 participant
Summary
src/internal/logger.tsas a dual-form{human, json}API. Default human mode is terse and prefix-free on stderr;LEADTYPE_LOG_FORMAT=json(or--jsonon CLI commands) emits one structured event per line for agents and CI;LEADTYPE_VERBOSE=1/--verbosefirehoses per-file events.process.stderr.writecallers (search/node.ts,remark/plugins/include.remark.ts,cli.ts) to the new logger; moves success summaries from stdout to stderr so pipelines stay clean.bun runpreambles sobun devfrom the example app emits ~4 lines instead of the prior 30+.What it looks like
Default (interactive terminal):
--json(agents, CI):--verbose/LEADTYPE_VERBOSE=1: per-file events with timing.Test plan
bun --cwd packages/leadtype run test— 98/98 passbun --cwd packages/leadtype run lint— clean (ultracite)bun --cwd packages/leadtype run check-types— clean (tsgo)bun --cwd packages/leadtype run buildsmoke — 2 lines on stderr, stdout emptyLEADTYPE_LOG_FORMAT=jsonsmoke — every stderr line parses as JSON with aneventkeyLEADTYPE_VERBOSE=1smoke — per-file events appearcd apps/example && bun devbuild phase — ~4 lines instead of ~30🤖 Generated with Claude Code