Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
📝 WalkthroughWalkthroughAdds an in-memory ring-buffer drain for evlog with write/read/clear APIs, a drain factory, query-string parsing, tests, package export/build wiring, docs, and a changeset declaring a minor release. ChangesMemory Adapter
Sequence DiagramsequenceDiagram
participant App
participant MemoryDrain
participant MemoryStore
participant ReadAPI
App->>MemoryDrain: send(context) / send(batch)
MemoryDrain->>MemoryStore: writeToMemory(events, {store,maxEvents})
MemoryStore->>MemoryStore: enforce maxEvents (evict oldest)
ReadAPI->>MemoryStore: readMemoryLogs(options)
MemoryStore->>ReadAPI: snapshot -> filtered & limited results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
Benchmark reportBundle size
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/content/3.integrate/adapters/self-hosted/03.memory.md`:
- Line 166: The variable name apiBrain is a typo in the named-store example;
rename it to a clear, consistent identifier (e.g., apiStore or apiMemory)
wherever it's declared and referenced so it matches createMemoryDrain({ store:
'api' }) and avoids confusion; update the declaration using createMemoryDrain
and any subsequent usages of apiBrain to the new name (search for apiBrain and
replace with apiStore/apiMemory in the example).
In `@packages/evlog/src/adapters/memory.ts`:
- Line 115: The read path is mutating state by calling
getOrCreateStore(storeName) when building events (e.g., the line creating const
events = [...getOrCreateStore(storeName)]) and in the other uses around 151-154;
change these reads to use a non-creating accessor (e.g., getStore or a new
readOnlyGetStore) that returns undefined or an empty array without inserting a
new entry into the in-memory map, and handle the undefined/empty result
accordingly so reads do not create persistent empty stores or grow memory with
arbitrary storeName values.
- Around line 136-137: The current check uses filtered.slice(-options.limit)
which treats 0 and negative limits incorrectly; update the logic around
options.limit to explicitly handle non-positive values: when options.limit !==
undefined and options.limit <= 0 return an empty array, otherwise if
options.limit < filtered.length return filtered.slice(-options.limit), else
return filtered. Refer to the options.limit check and the filtered.slice call in
the memory adapter (the block that currently does "if (options.limit !==
undefined && filtered.length > options.limit) { return
filtered.slice(-options.limit) }") and implement the explicit <= 0 guard before
calling slice.
🪄 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: 20e309d0-e0f9-40e5-b886-6402e3172053
⛔ Files ignored due to path filters (1)
packages/evlog/test/toolkit/__snapshots__/api-surface.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
.changeset/add-memory-drain-adapter.mdapps/docs/content/3.integrate/adapters/01.overview.mdapps/docs/content/3.integrate/adapters/self-hosted/03.memory.mdpackages/evlog/package.jsonpackages/evlog/src/adapters/memory.tspackages/evlog/test/adapters/memory.test.tspackages/evlog/tsdown.config.ts
- add parseReadMemoryLogsQuery() to coerce HTTP query-string params into ReadMemoryLogsOptions (works with Hono, h3/Nitro, Express, Fastify, Next.js, Elysia, NestJS) - fix readMemoryLogs: reads no longer mutate state via getOrCreateStore - fix readMemoryLogs: limit <= 0 now correctly returns an empty array - update changeset to document new helper - extend tests: 12 new cases covering all parseReadMemoryLogsQuery paths
- nitro.config.ts: defineConfig is not exported in nitro-nightly v4;
switch to `import type { NitroConfig } from 'nitro/types'` with a
`satisfies NitroConfig` annotation on the default export
- vite.config.ts: rollupConfig is not a valid NitroPluginConfig key
(the plugin accepts `config?: NitroConfig` but NitroConfig explicitly
omits rollupConfig); remove the unused Sentry externals config since
no @sentry package is present in this example
commit: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@examples/tanstack-start/nitro.config.ts`:
- Line 15: The double-cast "as unknown as NitroModuleSlot" bypasses type safety
for the evlog module registration; replace it with a single "as NitroModuleSlot"
cast (or better, declare a local typed adapter/interface matching
NitroModuleSlot and cast to that) and add a short comment documenting the exact
incompatibility between "evlog/nitro/v3" and "nitro-nightly" if one prevents a
direct type match, so the intent and remaining risk are explicit; update the
expression where the module is registered (the evlog registration expression
currently using the double-cast) to use the single cast or the new adapter type.
- Line 1: Replace the incorrect import of NitroConfig from 'nitro/types' with
the NitroConfig export from 'nitropack' so the type used in the NitroModuleSlot
derivation and the file-level "satisfies NitroConfig" expression is correct;
update the import statement that currently references NitroConfig and ensure any
references to NitroModuleSlot and the line using "satisfies NitroConfig"
continue to compile against the nitropack NitroConfig type, and do not remove or
change usage of defineConfig (keep using defineConfig from 'nitro' if present)
since defineConfig is exported in Nitro v4/nightly.
🪄 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: 0dfbb546-c92b-4396-9fa6-4d46a4e19629
📒 Files selected for processing (2)
examples/tanstack-start/nitro.config.tsexamples/tanstack-start/vite.config.ts
| @@ -1,12 +1,11 @@ | |||
| import { defineConfig } from 'nitro' | |||
| import type { NitroConfig } from 'nitro/types' | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Nitro or nitro-nightly export NitroConfig from 'nitro/types'? Is defineConfig not exported in Nitro v4?
💡 Result:
NitroConfig is NOT exported from 'nitro/types'. In Nitro v4, you should use defineNitroConfig/defineConfig from Nitro entrypoints (not a nitro/types re-export), and defineConfig is exported under the 'nitro' (or 'nitropack/...') module path—not absent. Evidence: - Nitro’s own configuration docs show importing defineConfig from "nitro": import { defineConfig } from "nitro" and using it to configure Nitro options [1]. - The Nitro configuration raw docs also show the same pattern for defineConfig usage [2]. - Nitro uses a separate defineNitroConfig import from nitropack/config, indicating config helpers live in those packages/paths rather than nitro/types [3]. So, if you’re seeing that “defineConfig not exported in Nitro v4”, that would be due to importing from the wrong module path (e.g., trying to import defineConfig from 'nitro/types' or an incorrect subpath). Use the documented imports: - import { defineConfig } from "nitro" [1] - or import { defineNitroConfig } from "nitropack/config" [3] Note: the searches surfaced Nitro’s type/config source files in the nitrojs/nitro repo, but they did not provide evidence that a public re-export named NitroConfig exists under the 'nitro/types' path. The authoritative docs instead point to nitro / nitropack/config for the supported configuration entrypoints [1][3].
Citations:
- 1: https://nitro.build/docs/configuration
- 2: https://nitro.build/raw/config.md
- 3: https://nitro-docs.pages.dev/config/
🏁 Script executed:
#!/bin/bash
# Verify the Nitro version and check package.json for nitro dependency
fd -t f 'package.json' examples/tanstack-start -x cat {} \; | jq -r '.dependencies + .devDependencies | to_entries[] | select(.key | contains("nitro")) | "\(.key): \(.value)"'Repository: HugoRCD/evlog
Length of output: 89
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="examples/tanstack-start/nitro.config.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
sed -n '1,120p' "$FILE" | cat -n
else
echo "Missing file: $FILE" >&2
exit 1
fiRepository: HugoRCD/evlog
Length of output: 739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "nitro/types|NitroConfig|defineConfig|defineNitroConfig" -S .Repository: HugoRCD/evlog
Length of output: 5840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,130p' packages/evlog/src/types.ts | cat -nRepository: HugoRCD/evlog
Length of output: 5023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "interface NitroConfig|type NitroConfig|NitroConfig\s*=" -S .
rg -n "declare module 'nitro/types'" -S .
rg -n "NitroConfig" examples/tanstack-start -SRepository: HugoRCD/evlog
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "NitroConfig" -S .
rg -n "nitro/types" -S .
sed -n '1,120p' packages/nuxthub/src/module.ts | cat -n
sed -n '1,80p' packages/evlog/src/nuxt/module.ts | cat -n
sed -n '1,120p' examples/tanstack-start/nitro.config.ts | cat -nRepository: HugoRCD/evlog
Length of output: 10767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== examples/tanstack-start/package.json =="
cat examples/tanstack-start/package.json
echo
echo "== examples/tanstack-start/tsconfig.json (if present) =="
ls examples/tanstack-start | rg -n "tsconfig"
if [ -f examples/tanstack-start/tsconfig.json ]; then
cat examples/tanstack-start/tsconfig.json
fi
echo
echo "== packages/evlog/README.md (Nitro/NitroConfig related section) =="
sed -n '160,240p' packages/evlog/README.md | cat -n
echo
echo "== packages/evlog/tsdown.config.ts (nitro/types mention context) =="
rg -n "nitro/types" packages/evlog/tsdown.config.ts
sed -n '60,100p' packages/evlog/tsdown.config.ts | cat -nRepository: HugoRCD/evlog
Length of output: 5810
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' packages/evlog/src/nitro-v3/module.ts | cat -n
sed -n '1,120p' packages/evlog/src/nitro-v3/plugin.ts | cat -nRepository: HugoRCD/evlog
Length of output: 7955
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' packages/evlog/test/nitro-v3/fixture/nitro.config.ts | cat -nRepository: HugoRCD/evlog
Length of output: 888
Fix NitroConfig type import in examples/tanstack-start/nitro.config.ts
NitroConfigis imported fromnitro/types, but this repo usesNitroConfigfromnitropackand Nitro’s public config/type exports don’t exposeNitroConfigvianitro/types; this will break typing at theNitroModuleSlotderivation andsatisfies NitroConfigusage.- Replace the
NitroConfigimport accordingly (line 1; used at lines 6 and 17). - The “
defineConfigis non-exported” rationale is also incorrect for Nitro v4/nightly:defineConfigis exported fromnitro(so switching away from it isn’t necessary).
🤖 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 `@examples/tanstack-start/nitro.config.ts` at line 1, Replace the incorrect
import of NitroConfig from 'nitro/types' with the NitroConfig export from
'nitropack' so the type used in the NitroModuleSlot derivation and the
file-level "satisfies NitroConfig" expression is correct; update the import
statement that currently references NitroConfig and ensure any references to
NitroModuleSlot and the line using "satisfies NitroConfig" continue to compile
against the nitropack NitroConfig type, and do not remove or change usage of
defineConfig (keep using defineConfig from 'nitro' if present) since
defineConfig is exported in Nitro v4/nightly.
| @@ -15,4 +14,4 @@ export default defineConfig({ | |||
| env: { service: 'tanstack-start-example' }, | |||
| }) as unknown as NitroModuleSlot, | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Double-cast escape hatch bypasses type safety.
The as unknown as NitroModuleSlot pattern completely disables type checking for the evlog module registration. While the comment explains the version mismatch between evlog/nitro/v3 and nitro-nightly, this loses all compile-time guarantees that the module shape is correct.
Consider whether a single as NitroModuleSlot cast would suffice, or document the specific incompatibility that requires the double-cast.
🤖 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 `@examples/tanstack-start/nitro.config.ts` at line 15, The double-cast "as
unknown as NitroModuleSlot" bypasses type safety for the evlog module
registration; replace it with a single "as NitroModuleSlot" cast (or better,
declare a local typed adapter/interface matching NitroModuleSlot and cast to
that) and add a short comment documenting the exact incompatibility between
"evlog/nitro/v3" and "nitro-nightly" if one prevents a direct type match, so the
intent and remaining risk are explicit; update the expression where the module
is registered (the evlog registration expression currently using the
double-cast) to use the single cast or the new adapter type.
2 tasks
This was referenced May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description
📝 Checklist
Summary by CodeRabbit
New Features
Documentation
Tests
Chores