feat(cli): add evlog init and gate on regressions with map --baseline - #459
Conversation
Wires evlog into an existing app: detects the framework, installs the package with the manager the lockfile implies, registers the integration, and writes a development-only .evlog/logs sink. Covers Nuxt, Nitro, Next.js, and TanStack Start. Config files are patched at the exact AST offsets oxc reports rather than reprinted from the tree, so comments, quote style, and formatting survive. Nothing that already exists is overwritten — there is no --force — and anything that cannot be spliced safely (a computed `modules` array, a TanStack `__root.tsx`) comes back as a manual step with the snippet and the reason. Running it twice is a no-op. The sink is guarded by import.meta.dev: a drain that writes files is a development convenience, and turning one on in production is not a decision a setup command should make on its own. Docs land at /cli/init, which renumbers the CLI content tree. URLs are derived from the slug, so no redirects are needed.
--min-score asks whether an app is good enough; --baseline asks whether this pull request made it worse. The comparison is per requirement rather than per score: a refactor that instruments one route and breaks another leaves the total untouched, and a gate watching only the number calls that a no-op. Silencing a passing check with a disable comment counts the same as breaking it — it is the cheapest way to turn a gate green without writing any instrumentation. New dark entry points are reported but do not fail: on an app that is not green yet, failing every pull request that adds an endpoint is how a team learns to turn the job off, and --min-score is the bar for new work. The baseline is a file the repository already contains, read from disk or through git show, so CI needs no network, no token, and no repository access — a private repo gates exactly like a public one. A run that reports a regression leaves evlog.map.json alone. Rewriting it there would move the ratchet down to the worse state, and the same command run again would report no regression and exit 0.
Setup is a conversation, not a form: init now asks for the service name, fuzzy-searches a destination across every adapter evlog ships, offers batching, enrichers, sampling and the Vite plugin as a multi-select, and shows the exact file list before writing anything. Built on @clack/prompts. One catalog drives both surfaces. The prompts render from it and --drain / --extras validate against it, so the interactive flow and its flags cannot drift into a state where an agent is unable to reproduce what a human just did. Every question has a flag; --yes, --json, a non-TTY, or CI selects the non-interactive path, and nothing can leave a caller waiting on a keystroke that is not coming. An unknown --drain stops the run rather than falling back, because silently wiring the wrong destination is worse than a failure. Secrets are never prompted for. init prints the environment variables the chosen adapter reads and leaves them alone — a command that asks for an API token is a command that writes a credential into a file it picked, and the answer lands in shell history on the way there. Only the filesystem drain is generated behind an import.meta.dev guard: it writes files on whatever box serves the request. Hosted destinations ship without one, which is the reason you chose them. Removes the separate install confirmation. The plan already lists the install command, and asking again after --no-install was passed is asking someone to repeat themselves.
`evlog init` writes files and `evlog map --baseline` needs a git history, so neither could be tried by hand against a real app without leaving a mess to undo. `pnpm cli:sandbox` builds throwaway apps under .sandbox/ — one per supported framework, each a git repo with evlog symlinked in — and prints the commands to run against them. `pnpm cli` runs the built binary from the root, so `--cwd .sandbox/nuxt` is the whole workflow. Three of the four apps are the existing `map` fixtures rather than new ones: they are already realistic apps with deliberately uneven instrumentation, which is what makes a map report worth reading. Nitro had no fixture and is generated. `--smoke` drives the whole feature matrix — 13 checks across 4 apps — non-interactively and reports what broke: JSON contracts, the --min-score and --baseline gates, init idempotence, drain wiring per destination, rejection of an unknown --drain, and that init never blocks on a prompt without a TTY. Each check gets a fresh copy of the app, because one that inherits the previous check's leftovers passes for the wrong reason.
init now runs the same analysis `map` runs and builds its offers from the result. An error catalog is offered when the same createError appears in more than one file, audit actions when sensitive entry points have no trail, the AI SDK and auth integrations when their packages are installed, batching when something actually leaves the process. Each offer carries the evidence that put it there, because "3 repeated errors found" says the tool read the project where a menu item asks for faith. The catalogs are seeded, not scaffolded. The generated error catalog holds the project's own repeated errors with the prose already written against them, and the audit catalog names an action per route the scan flagged — a to-do list with the types filled in rather than an empty example. Destinations are asked for twice, because they are two different problems: nobody sends local development traffic to Axiom and nobody reads production logs off the box's filesystem. Production takes several at once and fans the event out to each, branched on the environment in one plugin. Only the filesystem drain is gated to development, and batching wraps the network sends but never the local write, which would add latency to the one loop where the event is wanted on screen immediately. Enrichers and sampling stop being all-or-nothing: enrichers are picked individually and sampling is a rate profile. Sampling also lands in an evlog block that already exists — adding a key that is not there is the same offset splice as adding one at the root, and only a key that already exists is genuinely ambiguous. A drain file that already wires the same destinations is left alone rather than duplicated, which is what makes running init twice a no-op again; one that wires different destinations gets the new drain beside it under its own name, so asking for Axiom against an already-wired app no longer silently wires nothing. The adapter's variables are appended to .env.example and never .env — the example file is documentation, the real one holds secrets. The run finishes by executing doctor, so it answers "did it work" instead of naming another command. From a workspace root it sets up the apps rather than the root package, which serves no traffic. Redaction was considered and dropped: it is already on by default in production with value-pattern masking, so a picker of built-ins would have asked people to tick what is already ticked.
`pnpm cli:sandbox` already rebuilt the apps from scratch, which resets them but re-copies every fixture and re-inits git to undo a two-file change. Each app is already a git repo whose initial commit is the pristine state, so `--reset` restores it with a checkout and a clean instead. The symlink to evlog is part of that commit, so the reset puts it back rather than leaving the app unable to resolve the package — which is what a bare `git clean` would have done. Falls back to a full rebuild when the repo is missing.
Picking Axiom on a Next app that already had lib/evlog.ts reported "already exists" and wired the destination nowhere: the command asked a question and ignored the answer, then wrote .env.example promising keys no code read. An existing factory is now spliced into — drain, enrich and sampling added as options on the createEvlog call it already makes, with the imports and the statements they use inserted above it in that order. A file that does not call createEvlog, or one that already sets those keys, gets the snippet to paste and the reason instead. Either way the file itself is left byte for byte as it was. Also stops hiding enrichers and sampling from Next. createEvlog takes both `enrich` and `sampling`, and the enrichers read headers that its EnrichContext carries, so gating them to the Nitro-based frameworks excluded Next from two features it fully supports.
…ling presets init now sets the answers on its telemetry event: framework, development sink, sampling preset, one flag per destination, extra and enricher chosen, plus counts and whether an offer had anything behind it. That last one is the signal worth having — the gap between "the error catalog was offered" and "it was taken" is what says whether the offer earns its place. Strings go through the allowlist declared on the telemetry wrapper, so a value outside the catalog is dropped rather than sent. The service name is the one free-text answer in the flow and is never recorded, nor package names, paths, or anything read out of the user's source. The disclosure in the README and the telemetry page say so. The sampling presets now state the ratio in the label: "Balanced" told you nothing about what you were losing, where "1 in 4 info events" does. Two more sit at the ends of the range — "1 in 100 info events" for very high volume and "Only what went wrong" for warnings and errors alone. Errors stay at 100% in every preset, which the question says once rather than each option repeating. The generated catalog keeps the dashes in its wire prefix and camelCases the identifier, so a service called next-app-router-fixture no longer produces `nextapprouterfixtureErrors`. The sandbox apps gain two files throwing the same structured error, so the error-catalog offer has evidence to appear on, and the Next app now ships a real createEvlog factory instead of a re-export barrel — the barrel is a legitimate shape but the one init cannot splice into, so every sandbox run ended at "paste this snippet" and the patch path went unexercised by hand.
The ingest keeps a second allowlist of its own: whatever a client declares, the server stores only the custom keys named here, and `filterCustom` drops the rest. The list still held the three keys doctor sets, so every option init recorded arrived and was discarded — the run detail showed "no custom fields" while the CLI was sending twenty of them. Kept per-name rather than switching to an `init*` prefix rule. The client decides what to send and this decides what is worth storing; a namespace rule would accept a key nobody on this side has looked at, which is the property the allowlist exists for. Two hand-maintained lists on either side of an HTTP boundary drift, and the failure is silent — the option just never appears in the numbers. The test reads the CLI's own `initTelemetryFieldNames()` and fails on the first name this list does not cover.
The presets read as ratios — "1 in 4 info events" — which said what happened without saying who it was for. They are now named by the traffic the app takes: Everything, Low, Medium, High, Very high. Info is what moves across the ladder, since it is the bulk of both the volume and the bill, and warnings only give way at the top tier where halving them still leaves the shape. Debug is gone from every preset and from the generated config. An unspecified level is kept at 100%, and debug events only exist because somebody turned them on to chase something — a 5% sample of the logs you switched on to investigate a problem is a 5% chance of seeing the line you needed. Production builds strip log.debug() anyway, so there is rarely volume there to sample in the first place. Errors stay at 100% in every tier and are still written explicitly, so the invariant is visible in the file where somebody would otherwise wonder whether their sampling config is quietly dropping the events they page on.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Thank you for following the naming conventions! 🙏 |
📝 WalkthroughWalkthrough
ChangesCLI initialization
Map baseline gating
Documentation presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)apps/docs/app/assets/css/main.cssFile contains syntax errors that prevent linting: Line 4: Tailwind-specific syntax is disabled.; Line 9: Tailwind-specific syntax is disabled.; Line 24: Tailwind-specific syntax is disabled.; Line 28: Tailwind-specific syntax is disabled.; Line 33: Tailwind-specific syntax is disabled.; Line 47: Tailwind-specific syntax is disabled.; Line 60: Tailwind-specific syntax is disabled. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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/cli/src/commands/init.ts`:
- Around line 123-131: Update the no-selection branch in the init command to use
a cataloged INIT_NO_APPS error instead of constructing the JSON payload inline.
Add INIT_NO_APPS to cliErrors in errors.ts with the required why, fix, and link
fields, then throw it through the existing fail() flow so JSON output and
log.finding are handled consistently.
- Around line 133-147: Update the workspace execution loop in run() to handle
InitCancelled and EvlogError from each runInit call using the same
closeCancelled() and fail() paths as the single-app flow. Ensure cancellation
exits quietly after cleanup, while catalog errors render through the standard
failure handling instead of escaping as unhandled exceptions.
- Around line 99-105: The --apps selection logic silently ignores unmatched
names when at least one requested target matches. Update the requested/selected
handling in the init command to detect every entry that matches neither
target.label nor target.name, and stop the run with the existing unknown-value
error behavior before proceeding with setup.
In `@packages/cli/src/lib/init/catalog.ts`:
- Around line 138-164: Consolidate production eligibility around
PROD_DESTINATIONS: remove the hard-coded none exclusion from its filter and mark
the none destination as not production-safe, then update the production flag
validation in resolve.ts to check membership in PROD_DESTINATIONS rather than
special-casing its id. Keep DEV_DESTINATIONS behavior unchanged.
In `@packages/cli/src/lib/init/edit.ts`:
- Around line 77-79: Add JSDoc comments for the exported hasProperty function
and Splice interface in this file, matching the style and level of detail used
by neighboring public APIs. Document each symbol’s purpose and, where
applicable, its parameters and return value.
In `@packages/cli/src/lib/init/frameworks.ts`:
- Around line 399-403: Deduplicate destinations before the import-emission loop
in the relevant template generation flow, using the importer/drain identity such
as factory and specifier as the key. Ensure a destination selected for both dev
and prod produces only one import, while preserving distinct imports for
different destinations in nitroDrainTemplate() and nextFactoryParts().
- Around line 724-740: Update catalogKey so generated error keys that begin with
a digit are prefixed with a valid identifier segment before errorCatalogTemplate
emits them as bare object keys. Update quote to escape newline characters (and
corresponding carriage returns if applicable) so message and why values remain
valid string literals when generated. Preserve existing character normalization
and escaping behavior.
In `@packages/cli/src/lib/init/insight.ts`:
- Around line 153-161: Update the JSDoc example above auditActionName to match
the implementation’s output: use the plural `payments` resource and include the
appended verb, showing `payments.refund.created` for the given POST path.
In `@packages/cli/src/lib/init/prompts.ts`:
- Around line 155-156: In the prompt setup around availableExtras, call
input.offers(prodDrains, framework) once, assign the result to context, and pass
context to availableExtras instead of recomputing the identical value.
- Around line 322-333: Update canPrompt to use the context-owned ctx.tty value
instead of reading process.stdin.isTTY and process.stdout.isTTY directly.
Preserve the existing CI checks, and ensure prompting is disabled whenever
ctx.tty is false, consistent with the context definition that reflects stdout or
stderr interactivity.
In `@packages/cli/src/lib/init/resolve.ts`:
- Around line 117-142: Avoid duplicating applicable-extra computation between
resolveAnswers and droppedExtras. Extract the shared input.offers-based Set
construction into an applicableExtras helper and reuse it in both functions,
preserving the existing filtering and answer behavior.
- Around line 70-87: Use dedicated CLI error entries in parseEnrichersArg and
parseSamplingArg instead of INIT_INVALID_EXTRA, with messages and metadata
naming --enrichers and --sampling and accurately describing each input format.
In packages/cli/src/lib/init/resolve.ts lines 70-87, update both throw sites
accordingly; in packages/cli/src/commands/init.ts lines 24-30, add an
init-specific INIT_INVALID_FRAMEWORK entry or generalize MAP_INVALID_FRAMEWORK
so its why/tags no longer attribute init failures to map.
In `@packages/cli/src/lib/init/run.ts`:
- Around line 64-74: Add JSDoc documentation to the exported InsightSummary and
VerifySummary interfaces, and to the exported frameworkDocs symbol referenced by
the review. Document each public API and its fields sufficiently to match the
existing documentation style used by InitOptions, InitResult, and runInit,
without changing their behavior.
- Around line 168-261: Ensure interactive dry runs render the planned creates,
updates, and skips by invoking the existing plan-display logic for interactive
&& dryRun after planWiring, without requiring an Apply confirmation or executing
changes. Preserve the current confirmPlan flow for interactive non-dry-run
executions and avoid printing the plan more than once.
In `@packages/cli/src/lib/map/baseline.ts`:
- Around line 145-155: The baseline gate should not treat newly added dark
routes as regressions through the global score delta. Update compareToBaseline
or hasRegressed so comparison.delta is calculated only from routes present in
the baseline, or remove the global delta condition from the gate; preserve
per-check pass→fail and pass→suppressed regression detection while still
reporting added routes without failing.
- Around line 156-202: Update compareToBaseline so fixed entries for
fail-to-pass checks do not include the CheckRegression to field, changing the
fixed collection type and returned BaselineComparison typing to use a type such
as Omit<CheckRegression, 'to'>. Preserve the existing regression entries with
to: 'fail' or to: 'suppressed', while pushing only entry into fixed.
- Around line 140-143: Update isDark to derive darkness through
classifyRouteObservability(route) === 'dark' rather than evaluating raw check
statuses, ensuring exempt newly added routes remain classified in the canonical
exempt bucket.
In `@packages/cli/test/init.resolve.test.ts`:
- Around line 148-153: Update the CI case in the canPrompt test to stub both
process.stdin.isTTY and process.stdout.isTTY as true, matching the neighboring
terminal test, before asserting CI values return false. Keep the existing
CI=true and CI=1 assertions unchanged so the test exercises the CI guard rather
than the TTY guard.
- Around line 31-39: Add a focused test for parseProdDrainsArg that passes the
filesystem drain value and asserts it throws the production-safety validation
error, preserving coverage that file sinks cannot be configured for production.
In `@scripts/cli-sandbox.mjs`:
- Around line 157-177: Update newestSourceTime and readdirSafe to traverse
directories using node:fs readdirSync with withFileTypes instead of spawning ls
via execFileSync. Use each Dirent’s isDirectory() result to recurse and statSync
only files for mtimeMs, preserving the existing newest timestamp behavior and
avoiding per-directory subprocesses on Windows.
🪄 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 Plus
Run ID: 21bf8e3f-1cbf-48a5-801f-47596c3bae81
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
.changeset/cli-init-command.md.changeset/cli-map-baseline.md.gitignoreapps/docs/content/3.cli/0.overview.mdapps/docs/content/3.cli/1.init.mdapps/docs/content/3.cli/2.map.mdapps/docs/content/3.cli/3.rules.mdapps/docs/content/3.cli/4.scoring.mdapps/docs/content/3.cli/5.ci.mdapps/docs/content/3.cli/6.doctor.mdapps/docs/content/3.cli/7.telemetry.mdapps/telemetry/server/utils/allowed-tools.tsapps/telemetry/test/allowed-tools.test.tspackage.jsonpackages/cli/README.mdpackages/cli/package.jsonpackages/cli/src/commands/index.tspackages/cli/src/commands/init.tspackages/cli/src/commands/map.tspackages/cli/src/index.tspackages/cli/src/lib/command.tspackages/cli/src/lib/errors.tspackages/cli/src/lib/init/catalog.tspackages/cli/src/lib/init/edit.tspackages/cli/src/lib/init/frameworks.tspackages/cli/src/lib/init/insight.tspackages/cli/src/lib/init/pm.tspackages/cli/src/lib/init/prompts.tspackages/cli/src/lib/init/report.tspackages/cli/src/lib/init/resolve.tspackages/cli/src/lib/init/run.tspackages/cli/src/lib/init/telemetry.tspackages/cli/src/lib/init/workspace.tspackages/cli/src/lib/map/baseline.tspackages/cli/src/lib/map/report.tspackages/cli/src/lib/map/write.tspackages/cli/test/init.insight.test.tspackages/cli/test/init.resolve.test.tspackages/cli/test/init.telemetry.test.tspackages/cli/test/init.test.tspackages/cli/test/map/baseline.test.tsscripts/cli-sandbox.mjs
Correctness: - The baseline gate no longer contradicts its own rule. `delta` was `current.score - baseline.score`, a weighted average over every route, so a new dark endpoint dragged it below zero and failed the gate that new dark endpoints are documented not to fail. It is now the movement across the entry points that existed in both maps; the global difference stays as `totalDelta` for the report, which needs the arithmetic between the two numbers it shows. - `fixed` entries no longer carry `to: 'fail'`. They are their own type, so a consumer of `evlog map --json` cannot read a repaired check as a broken one. - Added routes use the scan's own classifier, so an exempt route — a health probe, a page that fetches nothing — stays exempt instead of being listed as needing instrumentation it was excused from. - The generated error catalog is always parseable: a key that does not start like an identifier is quoted, and newlines inside a message are escaped rather than ending the string literal mid-file. - Drain imports are deduped, so the same destination chosen for development and production emits one import rather than two. - `--apps web,shopp` stops on `shopp` instead of setting up `web` and saying nothing, which is the silent-default behaviour every other flag here refuses. - Ctrl-C inside a workspace run ends the loop quietly; apps already set up keep what they got and the rest are untouched. An error mid-loop goes through the same reporting as everywhere else. - Interactive `--dry-run` prints the plan again. The confirm step was the only thing that rendered it and dry runs skip the confirm, so the terminal showed the questions and then nothing. Its outro no longer claims the app was wired. Clarity: - `--enrichers`, `--sampling`, `--framework` and `--apps` have catalog entries of their own. Borrowing `INIT_INVALID_EXTRA` told people their `--extras` was wrong when it was not, and hand-rolling `cli.INIT_NO_APPS` meant that one failure carried no why or fix. - `canPrompt` reads the context instead of `process`, with stdin as its own field: "can it be styled" and "is anyone there to answer" are two questions, and the tests now say which they mean rather than patching globals. - `PROD_DESTINATIONS` is the only place the filesystem sink is excluded from production; `resolveAnswers` and `droppedExtras` share one applicable set. - The sandbox walks directories with `node:fs` rather than one `ls` per directory, which also stops it silently rebuilding on every run on Windows. Tests cover the new-dark-route case that motivated the delta change, the production-safety gate on `--prod-drain`, and every `canPrompt` branch. The git fallback case gets explicit headroom: four git invocations under parallel load is not a 5s operation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/lib/map/baseline.ts (1)
141-151: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winOnly fall back to Git when the working-copy map is absent.
This
catchalso catchesparseMapFilefailures and read-permission errors. A malformedevlog.map.jsoncan therefore silently compare againstgit:HEADinstead of returningMAP_BASELINE_INVALID, undermining the selected baseline. Restrict the fallback toENOENT; parse after the read succeeds.🤖 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/cli/src/lib/map/baseline.ts` around lines 141 - 151, The baseline-loading flow should fall back to Git only when reading the working-copy map fails with an ENOENT error. In the try/catch around readFileSync, handle other filesystem errors by propagating them, and call parseMapFile only after the read succeeds so malformed content returns MAP_BASELINE_INVALID instead of using git:HEAD.
🤖 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/cli/src/commands/init.ts`:
- Around line 139-144: The zero-selection branch in init must not reuse
INIT_NO_APPS with the placeholder value “nothing.” Update
packages/cli/src/commands/init.ts lines 139-144 to use a dedicated zero-apps
error, and add its catalog entry in packages/cli/src/lib/errors.ts lines 113-121
with wording that does not imply an unknown user-entered app name.
---
Outside diff comments:
In `@packages/cli/src/lib/map/baseline.ts`:
- Around line 141-151: The baseline-loading flow should fall back to Git only
when reading the working-copy map fails with an ENOENT error. In the try/catch
around readFileSync, handle other filesystem errors by propagating them, and
call parseMapFile only after the read succeeds so malformed content returns
MAP_BASELINE_INVALID instead of using git:HEAD.
🪄 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 Plus
Run ID: 41968259-93af-4b60-92ce-2df930f82e53
📒 Files selected for processing (15)
packages/cli/src/commands/init.tspackages/cli/src/core/context.tspackages/cli/src/lib/errors.tspackages/cli/src/lib/init/catalog.tspackages/cli/src/lib/init/edit.tspackages/cli/src/lib/init/frameworks.tspackages/cli/src/lib/init/insight.tspackages/cli/src/lib/init/prompts.tspackages/cli/src/lib/init/resolve.tspackages/cli/src/lib/init/run.tspackages/cli/src/lib/map/baseline.tspackages/cli/src/lib/map/report.tspackages/cli/test/init.resolve.test.tspackages/cli/test/map/baseline.test.tsscripts/cli-sandbox.mjs
| if (selected.length === 0) { | ||
| return fail( | ||
| cliErrors.INIT_NO_APPS({ value: 'nothing', known: targets.map(app => app.label).join(', ') }), | ||
| { args, log, ui }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
INIT_NO_APPS message template doesn't fit the "zero apps selected" case. The template assumes value is a literal string the user typed, but the "nothing interactively selected" call site passes the placeholder 'nothing', producing a confusing rendered message ("No workspace app matches "nothing"") for users who never typed that value.
packages/cli/src/commands/init.ts#L139-L144: stop passing the literal'nothing'asvalue; use a dedicated message/branch for "you selected zero apps" instead of reusing the unknown-value error shape.packages/cli/src/lib/errors.ts#L113-L121: add a distinct catalog entry (or parameterizeINIT_NO_APPSto accept a "none selected" mode) so the wording doesn't imply the user typed an unrecognized name.
📍 Affects 2 files
packages/cli/src/commands/init.ts#L139-L144(this comment)packages/cli/src/lib/errors.ts#L113-L121
🤖 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/cli/src/commands/init.ts` around lines 139 - 144, The zero-selection
branch in init must not reuse INIT_NO_APPS with the placeholder value “nothing.”
Update packages/cli/src/commands/init.ts lines 139-144 to use a dedicated
zero-apps error, and add its catalog entry in packages/cli/src/lib/errors.ts
lines 113-121 with wording that does not imply an unknown user-entered app name.
The map section stuttered for the first half second, which is exactly when the score counts up, twenty-nine bars rise and three coverage bars fill. Three things were competing for that budget. The skyline animated `height` and the coverage bars animated `width`, both layout properties: twenty-nine siblings resizing inside one flex row makes the browser lay the whole row out again on every frame of every bar. They scale instead — `transform` is composited and touches neither layout nor paint — and `transition-transform` replaces `transition-all` so the browser stops watching properties nobody animates. The glow behind each title is a blurred, text-shadowed duplicate of the heading with `animate-pulse` on it. Every step of that animation re-rasterized a 24px shadow through a filter over the whole heading, and CSS animations do not stop when an element scrolls out of view — so all thirteen titles on the page were paying it at once, all the time. They now share a `.title-glow` class that puts each on its own compositing layer: the blur is rasterized once and the pulse only changes the layer's opacity. It also honours prefers-reduced-motion, which `animate-pulse` did not. The score counted up on a 24ms interval, landing between frames — some frames rendered the same number twice, others skipped one, visible as stutter beside the bars. It runs off `requestAnimationFrame` now, one update per frame, and pauses with the tab. The class carries the effect only. The header logo glows at its own size, so typography stays in the markup where each caller sets it.
The score column had an intrinsic width and the row a wider gap than the rows below it, so the skyline began on a vertical of its own — one that moved with whatever the font rendered. It now matches the coverage labels exactly: same width, same gap, so both start on the same line. Both columns go to w-32 rather than w-28, since they have to stay equal and "Middleware & jobs" was truncating at the narrower width.
4b46239 to
c646dc1
Compare
Most of them restated what the line below already said. What is left is the handful that record a decision someone would otherwise undo: why the baseline delta is not the global score difference, why debug is never sampled, why .env.example and never .env, why the ingest allowlist has to track the CLI's.
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 `@apps/docs/app/components/features/FeatureCliMap.vue`:
- Around line 195-200: Remove the HTML comment blocks in the FeatureCliMap Vue
template, including the additional block around the referenced later section.
Preserve any necessary rationale by moving it to an appropriate CSS, script, or
documentation comment without leaving HTML comments in the template.
🪄 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 Plus
Run ID: 62fcbad2-e9ff-4d17-a4e9-5b54b35a6635
📒 Files selected for processing (14)
apps/docs/app/assets/css/main.cssapps/docs/app/components/AppHeaderLogo.vueapps/docs/app/components/LandingHero.vueapps/docs/app/components/features/FeatureAdapters.vueapps/docs/app/components/features/FeatureAgentReady.vueapps/docs/app/components/features/FeatureAiSdk.vueapps/docs/app/components/features/FeatureAudit.vueapps/docs/app/components/features/FeatureCliMap.vueapps/docs/app/components/features/FeatureClientDrain.vueapps/docs/app/components/features/FeatureFrameworks.vueapps/docs/app/components/features/FeaturePerformance.vueapps/docs/app/components/features/FeatureSampling.vueapps/docs/app/components/features/FeatureSimpleApi.vueapps/docs/app/components/features/FeatureStructuredErrors.vue
| <!-- | ||
| The score column matches the coverage labels below — same width, | ||
| same gap — so the skyline starts on the same vertical as the | ||
| coverage tracks. Left to its intrinsic width it lined up with | ||
| nothing, and moved with the font. | ||
| --> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove HTML comments from the Vue template.
These newly added <!-- ... --> blocks violate the repository rule for **/*.vue. Move the rationale to CSS, script comments, or documentation instead.
Also applies to: 214-220
🤖 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 `@apps/docs/app/components/features/FeatureCliMap.vue` around lines 195 - 200,
Remove the HTML comment blocks in the FeatureCliMap Vue template, including the
additional block around the referenced later section. Preserve any necessary
rationale by moving it to an appropriate CSS, script, or documentation comment
without leaving HTML comments in the template.
Source: Coding guidelines
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 (3)
packages/cli/src/lib/map/baseline.ts (1)
124-132: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not fall back to Git when the default local map is invalid.
parseMapFileruns inside this catch, so invalid JSON or a malformedevlog.map.jsonis treated like a missing file and compared againstgit:HEAD. Only catchreadFileSyncerrors for the local fallback; parse successful reads outside the catch.🤖 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/cli/src/lib/map/baseline.ts` around lines 124 - 132, The baseline loading flow around readFileSync and parseMapFile currently treats malformed local maps as missing files. Restrict the try/catch to readFileSync errors, then parse the successfully read raw content outside the catch; retain the git:HEAD fallback only when the local file cannot be read.Source: Coding guidelines
packages/cli/src/lib/init/run.ts (1)
268-276: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNo partial-write handling in the plan-apply loop.
If
writeFile/mkdirfails partway throughplan.actions, the exception propagates uncaught, leaving earlier actions already written to disk with no summary of what succeeded before the failure. The user approved the whole plan but may be left with an undocumented partially-applied one.Wrap each action (or the loop) to collect per-action outcomes and surface which files were written before the failure, rather than letting a mid-loop exception silently leave partial state.
🛠️ Illustrative direction
if (!dryRun) { await log.step('write', async () => { + const done: string[] = [] for (const action of plan.actions) { - await mkdir(dirname(action.path), { recursive: true }) - await writeFile(action.path, action.contents, 'utf8') + try { + await mkdir(dirname(action.path), { recursive: true }) + await writeFile(action.path, action.contents, 'utf8') + done.push(action.relative) + } catch (error) { + throw new Error(`Failed writing ${action.relative} after writing ${done.length}/${plan.actions.length} planned files: ${(error as Error).message}`) + } } return plan.actions.length }) }🤖 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/cli/src/lib/init/run.ts` around lines 268 - 276, Update the plan application loop inside the write step to handle mkdir/writeFile failures explicitly, recording each action’s success or failure and the files written before any error. Surface a concise summary of these per-action outcomes when the loop cannot complete, while preserving the existing successful return of plan.actions.length.packages/cli/src/lib/init/frameworks.ts (1)
556-572: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winImport-merge check only looks at the specifier, not the actual named imports — can generate a file that throws
ReferenceError.
missing(line 661-664) is computed viahasImportFrom, which only checks whether any import already comes from the target specifier. Iflib/evlog.tsalready imports something (anything) from'evlog/enrichers'unrelated to theenrichoption, the new enrichers import is skipped entirely — butnextFactoryPartsstill emits anenrichers = [...]array referencing all selected enricher factory names. Any name not already imported becomes an undefined reference at runtime.Track actually-imported names per specifier (not just specifier presence) so only the missing named imports are added.
Also applies to: 657-673
🤖 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/cli/src/lib/init/frameworks.ts` around lines 556 - 572, The import-merge logic around the enrichers handling must track imported symbols, not merely whether a specifier is present. Update the logic using hasImportFrom and missing so it parses existing named imports from each specifier, computes which selected enricher factory names are absent, and adds only those missing names while preserving existing imports and ensuring every name emitted by nextFactoryParts is defined.
🤖 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/cli/src/lib/init/frameworks.ts`:
- Around line 556-572: The import-merge logic around the enrichers handling must
track imported symbols, not merely whether a specifier is present. Update the
logic using hasImportFrom and missing so it parses existing named imports from
each specifier, computes which selected enricher factory names are absent, and
adds only those missing names while preserving existing imports and ensuring
every name emitted by nextFactoryParts is defined.
In `@packages/cli/src/lib/init/run.ts`:
- Around line 268-276: Update the plan application loop inside the write step to
handle mkdir/writeFile failures explicitly, recording each action’s success or
failure and the files written before any error. Surface a concise summary of
these per-action outcomes when the loop cannot complete, while preserving the
existing successful return of plan.actions.length.
In `@packages/cli/src/lib/map/baseline.ts`:
- Around line 124-132: The baseline loading flow around readFileSync and
parseMapFile currently treats malformed local maps as missing files. Restrict
the try/catch to readFileSync errors, then parse the successfully read raw
content outside the catch; retain the git:HEAD fallback only when the local file
cannot be read.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 442548a4-34ea-4b0b-8e30-627790460041
📒 Files selected for processing (13)
apps/docs/app/assets/css/main.cssapps/docs/app/components/features/FeatureCliMap.vuepackages/cli/src/lib/init/catalog.tspackages/cli/src/lib/init/edit.tspackages/cli/src/lib/init/frameworks.tspackages/cli/src/lib/init/insight.tspackages/cli/src/lib/init/pm.tspackages/cli/src/lib/init/prompts.tspackages/cli/src/lib/init/resolve.tspackages/cli/src/lib/init/run.tspackages/cli/src/lib/init/telemetry.tspackages/cli/src/lib/init/workspace.tspackages/cli/src/lib/map/baseline.ts
Two commands' worth of work on
@evlog/cli, plus a sandbox to exercise it from the repo root.evlog initmapsays which entry points are dark;initis what you run before that. On a terminal it reads the project, asks what it cannot infer, and shows the plan before touching anything.It only offers what it can back up.
initruns the same analysismapruns, so the error catalog appears when the samecreateErroris written in more than one file, audit actions when sensitive entry points have no trail, the AI and auth integrations when their packages are installed. Each offer carries the evidence that put it there.The catalogs are seeded, not scaffolded. The generated file holds the project's own repeated errors, with the prose already written against them — a to-do list with the types filled in rather than an empty example.
It never overwrites. Configs are patched at the exact AST offsets oxc reports, so comments, quote style and formatting survive. A file that exists is patched where that is unambiguous and handed back as a snippet where it is not. Running it twice is a no-op.
Secrets are never prompted for. The adapter's variables are appended to
.env.example, never.env.Every prompt has a flag.
--yes,--json, a non-TTY orCIselects the non-interactive path, so an agent reproduces exactly what a human just did and can never be left waiting on a keystroke. An unknown--drainstops the run rather than silently defaulting.evlog map --baseline--min-scoreasks whether an app is good enough;--baselineasks whether this pull request made it worse. The comparison is per requirement rather than per score — a refactor that instruments one route and breaks another leaves the total untouched.Silencing a passing check with a disable comment counts the same as breaking it. New dark entry points are reported but do not fail: on an app that is not green yet, failing every PR that adds an endpoint is how a team learns to turn the job off.
The baseline is a file the repository already contains, read from disk or through
git show— no network, no token, no repository access, so it behaves the same on a private repo. A run that reports a regression leavesevlog.map.jsonalone rather than ratcheting the baseline down to the worse state.Sandbox
initwrites files and--baselineneeds a git history, so neither could be tried by hand without leaving a mess. Three of the four apps are the existingmapfixtures; Nitro is generated.Telemetry
initrecords which options were picked — framework, destinations, extras, sampling tier, counts, and whether an offer had anything behind it. That last one is the signal worth having: the gap between "the error catalog was offered" and "it was taken" says whether the offer earns its place.Every string is an id from the CLI's own catalog behind an allowlist. The service name is never recorded, nor package names, paths, or anything read out of the user's source. The ingest keeps a second allowlist of its own, which is updated here — and a test reads the CLI's field list so the two cannot drift silently.
Notes for review
pnpm-lock.yamldiff is mostly ambient.pnpm install --lockfile-onlyonmainalone produces ~997 changed lines; roughly 50 of them here are the@clack/promptsaddition./cli/initpage, a ratchet section in/cli/ci, and the CLI content tree renumbered. URLs derive from the slug, so no redirects are needed.Summary by CodeRabbit
evlog initfor interactive or fully non-interactive setup (framework wiring, dev/prod destinations, optional extras, andevlog doctorverification).evlog map --baselineto detect regressions vs committedevlog.map.json, including CI gate behavior (supportsgit:<ref>).init,map --baseline, rules/scoring, CI semantics, telemetry, and the CLI overview.