refactor: implement structured logging #48
Conversation
…gging in knowledge plugin
WalkthroughThe changes introduce comprehensive structured logging throughout the plugin-knowledge module by adding source context and agent identifiers to log entries. Runtime context is threaded through all embedding and text-generation functions via a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/service.ts (1)
856-867: <!-- [scratchpad_start]
[task review_verification 1/1]
Found @elizaos/core version 1.6.4. Now search for any documentation or changelog.[observations]
- @elizaos/core version is 1.6.4
- Could search for online documentation or look for changelogs
- But the key evidence is already clear from the code
[critical_evidence_summary]
- Service code (src/service.ts:866): calls this.runtime.countMemories(roomId, unique, tableName) - 3 parameters
- Test mock (src/tests.ts:27): async countMemories(roomId: UUID) - 1 parameter only
- Comment (src/service.ts:861): "runtime.countMemories expects (roomId, unique, tableName) as positional parameters"
- These are INCONSISTENT
[analysis]
The review comment is correct to ask for verification. The evidence shows:
- The service implementation passes 3 positional parameters
- The test mock only accepts 1 parameter
- These are incompatible
This indicates either:
- The comment is wrong (runtime doesn't actually expect those 3 params)
- The service code is wrong (shouldn't be passing 3 params)
- The test mock is incomplete
Given that I cannot access the actual @elizaos/core source in the sandbox, I should verify using available information. The test mock clearly contradicts what the service code is doing, which is a red flag.
[conclusion]
The review comment's concern is VALID and SUBSTANTIATED by evidence. The signature mismatch between test mock and service usage is a real discrepancy that needs to be resolved. This is not just unverified - it's contradicted by the test code.[done]
The review comment should be rewritten to highlight the actual discrepancy found: the test mock only accepts roomId, but the service passes 3 arguments.</scratchpad_end] -->
Based on my verification, I've found a critical discrepancy that confirms the review comment's concern:
The test mock in
src/tests.ts(line 27) shows:async countMemories(roomId: UUID) { return Array.from(memories.values()).filter((m) => m.roomId === roomId).length; }But the service code in
src/service.ts(line 866) calls:this.runtime.countMemories(roomId, unique, tableName);The test mock only accepts a single
roomIdparameter, but the service code attempts to pass three positional parameters(roomId, unique, tableName). This is a direct contradiction that indicates either the service implementation or the test mock is incorrect.
The
runtime.countMemoriessignature does not match the service implementation.The test mock in
src/tests.ts:27accepts only a singleroomIdparameter:async countMemories(roomId: UUID), but the service calls it with three positional arguments on line 866:this.runtime.countMemories(roomId, unique, tableName). This inconsistency suggests either the comment on line 861 is incorrect about the expected signature, or the service is passing arguments that the runtime doesn't accept. Verify the actualIAgentRuntime.countMemoriessignature in@elizaos/coreto resolve this mismatch.src/document-processor.ts (2)
132-201: Pass runtime intogetProviderRateLimitsto honor per‑agent settings
processFragmentsSynchronouslycallsgetProviderRateLimits()withoutruntime, so rate limits are derived only from env/defaults, not from any runtime-specific overrides you may have configured.You likely want:
- const providerLimits = await getProviderRateLimits(); + const providerLimits = await getProviderRateLimits(runtime);Same applies to the call in
generateContextsInBatch(line 636). This keeps behavior consistent with the new runtime-aware config.
133-189: Address ESLintobject-shorthandin this fileStatic analysis flags several logger contexts (e.g.,
agentId: agentId,documentId: documentId) for not using shorthand. Switching to shorthand where possible will satisfy lint and slightly reduce noise:- logger.warn({ src: 'plugin:knowledge', agentId: agentId, documentId }, 'No text content available to chunk'); + logger.warn({ src: 'plugin:knowledge', agentId, documentId }, 'No text content available to chunk');Apply similarly at the other lines called out by ESLint.
src/config.ts (2)
194-256: FixgetProviderRateLimitshelper to truly “fallback” toprocess.envIn
getProviderRateLimits, the localgetSettingis documented as “get setting from runtime or fallback to process.env”, but whenruntimeis present it currently ignoresprocess.env:const getSetting = (key: string, defaultValue: string) => { if (runtime) { return runtime.getSetting(key) || defaultValue; } return process.env[key] || defaultValue; };This means env-configured rate limits are silently ignored if the runtime doesn’t define the keys. To preserve expected behavior, you likely want:
- const getSetting = (key: string, defaultValue: string) => { - if (runtime) { - return runtime.getSetting(key) || defaultValue; - } - return process.env[key] || defaultValue; - }; + const getSetting = (key: string, defaultValue: string) => { + if (runtime) { + return runtime.getSetting(key) || process.env[key] || defaultValue; + } + return process.env[key] || defaultValue; + };so runtime settings override env, but env still applies when runtime is silent.
128-182: Clean up ESLintobject-shorthandusage in config logsSeveral logger context objects in this file use explicit
agentId: runtime?.agentIdormodelName: modelName. ESLint flags these underobject-shorthand. Where the property name matches the variable, you can shorten:- logger.debug({ src: 'plugin:knowledge', agentId: runtime?.agentId }, 'CTX validation: Checking text generation settings'); + logger.debug({ src: 'plugin:knowledge', agentId: runtime?.agentId }, 'CTX validation: Checking text generation settings');(Here only
srcdiffers; for others like{ modelName, provider: 'openrouter' }you can omit the explicitmodelName: modelName.) Apply this pattern at the lines noted in the static analysis hints.
🧹 Nitpick comments (8)
src/actions.ts (3)
84-84: Consider addingagentIdfor consistency with other files.Other files in this PR (e.g.,
src/service.ts,src/provider.ts) includeagentIdin structured logs whenruntimeis available. Adding it here would improve traceability.- logger.warn({ src: 'plugin:knowledge:action:process' }, 'Knowledge service not available'); + logger.warn({ src: 'plugin:knowledge:action:process', agentId: runtime.agentId }, 'Knowledge service not available');
199-199: Consider addingagentIdfor consistency.Same as above -
runtime.agentIdis available and would improve log traceability.- logger.error({ src: 'plugin:knowledge:action:process', error }, 'Error processing knowledge'); + logger.error({ src: 'plugin:knowledge:action:process', agentId: runtime.agentId, error }, 'Error processing knowledge');
329-329: Consider addingagentIdfor consistency.Same as above.
- logger.error({ src: 'plugin:knowledge:action:search', error }, 'Error searching knowledge'); + logger.error({ src: 'plugin:knowledge:action:search', agentId: runtime.agentId, error }, 'Error searching knowledge');src/routes.ts (3)
147-185: Consider including agent/world identifiers in top‑level error logsWithin the file-upload branch you already have agentId and worldId; top-level error handling (e.g., the catch at lines 329–335) only logs method/path. Including agentId/worldId there would improve traceability for upload failures without changing behavior.
349-407: Agent ID is logged but not used for filtering documentsYou now log
agentIdingetKnowledgeDocumentsHandler, butservice.getMemoriesis still called without a room/agent filter. If multi-tenant isolation is expected at this layer, consider threadingagentIdinto the query; if not, the logging might misleadingly suggest per-agent filtering.
869-1004: Graph nodes handler: logging is helpful but watch volumeThe graph-nodes endpoint now logs pagination parameters, fragments counts, sample fragment metadata, and final graph size with agentId. Functionally it’s unchanged, but the sample fragment metadata logs could become noisy in production; consider gating them behind
NODE_ENV !== 'production'like you did inexpandDocumentGraphHandler.src/document-processor.ts (1)
346-477: Chunk batch / embedding logs are accurate, but “all chunks processed” message can misleadBatch-level debug logs now carry batch numbers, ranges, and failure details; embedding and DB-save failures are clearly logged per chunk.
One small nit: the “All chunks processed successfully” info log fires when the last chunk is saved, even if earlier chunks failed (and were counted in
failedCount). Consider guarding it:- if (originalChunkIndex === chunks.length - 1) { + if (originalChunkIndex === chunks.length - 1 && failedCount === 0) {so it only emits on truly clean runs.
src/config.ts (1)
114-185: CTX and embedding validation logs are precise but may be noisy
validateConfigRequirementsnow logs when no EMBEDDING_PROVIDER is set, when CTX validation starts, and when contextual knowledge is disabled. Behavior (exceptions thrown on missing keys) matches prior logic, but note these logs will run every time config is validated; if that happens frequently per request, consider gating some of the info logs behind a higher log level or a one-time init.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (13)
eslint.config.js(1 hunks)package.json(3 hunks)src/actions.ts(3 hunks)src/config.ts(8 hunks)src/ctx-embeddings.ts(4 hunks)src/docs-loader.ts(5 hunks)src/document-processor.ts(25 hunks)src/documents-provider.ts(2 hunks)src/index.ts(1 hunks)src/llm.ts(30 hunks)src/provider.ts(1 hunks)src/routes.ts(44 hunks)src/service.ts(28 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/document-processor.ts (1)
src/utils.ts (1)
convertPdfToTextFromBuffer(121-160)
src/config.ts (2)
src/llm.ts (1)
ModelConfig(13-13)src/types.ts (1)
ModelConfig(56-56)
src/service.ts (1)
src/config.ts (1)
validateModelConfig(16-106)
src/llm.ts (1)
src/types.ts (1)
ModelConfig(56-56)
🪛 ESLint
src/document-processor.ts
[error] 133-133: Expected property shorthand.
(object-shorthand)
[error] 141-141: Expected property shorthand.
(object-shorthand)
[error] 146-146: Expected property shorthand.
(object-shorthand)
[error] 157-157: Expected property shorthand.
(object-shorthand)
[error] 182-182: Expected property shorthand.
(object-shorthand)
[error] 188-188: Expected property shorthand.
(object-shorthand)
[error] 387-387: Expected property shorthand.
(object-shorthand)
[error] 415-415: Expected property shorthand.
(object-shorthand)
[error] 424-424: Expected property shorthand.
(object-shorthand)
[error] 455-455: Expected property shorthand.
(object-shorthand)
[error] 462-462: Expected property shorthand.
(object-shorthand)
src/docs-loader.ts
[error] 43-43: Expected property shorthand.
(object-shorthand)
[error] 47-47: Expected property shorthand.
(object-shorthand)
[error] 53-53: Expected property shorthand.
(object-shorthand)
[error] 57-57: Expected property shorthand.
(object-shorthand)
[error] 77-77: Expected property shorthand.
(object-shorthand)
[error] 103-103: Expected property shorthand.
(object-shorthand)
[error] 106-106: Expected property shorthand.
(object-shorthand)
[error] 109-109: Expected property shorthand.
(object-shorthand)
[error] 115-115: Expected property shorthand.
(object-shorthand)
src/routes.ts
[error] 390-390: Expected property shorthand.
(object-shorthand)
[error] 406-406: Expected property shorthand.
(object-shorthand)
[error] 465-465: Expected property shorthand.
(object-shorthand)
[error] 470-470: Expected property shorthand.
(object-shorthand)
[error] 498-498: Expected property shorthand.
(object-shorthand)
[error] 833-833: Expected property shorthand.
(object-shorthand)
[error] 851-851: Expected property shorthand.
(object-shorthand)
[error] 862-862: Expected property shorthand.
(object-shorthand)
[error] 918-918: Expected property shorthand.
(object-shorthand)
[error] 934-934: Expected property shorthand.
(object-shorthand)
[error] 939-939: Expected property shorthand.
(object-shorthand)
[error] 965-965: Expected property shorthand.
(object-shorthand)
[error] 974-974: Expected property shorthand.
(object-shorthand)
[error] 985-985: Expected property shorthand.
(object-shorthand)
[error] 1002-1002: Expected property shorthand.
(object-shorthand)
[error] 1021-1021: Expected property shorthand.
(object-shorthand)
[error] 1029-1029: Expected property shorthand.
(object-shorthand)
[error] 1036-1036: Expected property shorthand.
(object-shorthand)
[error] 1040-1040: Expected property shorthand.
(object-shorthand)
[error] 1047-1047: Expected property shorthand.
(object-shorthand)
[error] 1064-1064: Expected property shorthand.
(object-shorthand)
[error] 1071-1071: Expected property shorthand.
(object-shorthand)
[error] 1078-1078: Expected property shorthand.
(object-shorthand)
[error] 1082-1082: Expected property shorthand.
(object-shorthand)
[error] 1089-1089: Expected property shorthand.
(object-shorthand)
[error] 1104-1104: Expected property shorthand.
(object-shorthand)
[error] 1107-1107: Expected property shorthand.
(object-shorthand)
[error] 1126-1126: Expected property shorthand.
(object-shorthand)
[error] 1136-1136: Expected property shorthand.
(object-shorthand)
[error] 1141-1141: Expected property shorthand.
(object-shorthand)
[error] 1148-1148: Expected property shorthand.
(object-shorthand)
[error] 1158-1158: Expected property shorthand.
(object-shorthand)
[error] 1166-1166: Expected property shorthand.
(object-shorthand)
[error] 1199-1199: Expected property shorthand.
(object-shorthand)
[error] 1208-1208: Expected property shorthand.
(object-shorthand)
🔇 Additional comments (44)
eslint.config.js (1)
1-12: LGTM! Clean ESLint configuration.The setup correctly extends the ElizaOS plugin configuration, which includes the structured logging rule mentioned in the PR objectives. The flat config format is appropriate for ESLint 9.
package.json (3)
4-4: LGTM! Appropriate version bump.The minor version increment is suitable for the structured logging refactor and tooling additions.
75-76: LGTM! Lint workflow improvements align with PR objectives.The updated lint script properly runs ESLint with auto-fix before Prettier, and the new
lint:checkscript provides CI-friendly validation without modifications.
50-51: ESLint 9 compatibility with typescript-eslint packages is confirmed.The versions specified are compatible:
@typescript-eslint/eslint-plugin@^8.22.0and@typescript-eslint/parser@^8.22.0explicitly support ESLint 9 via their peer dependency range (^8.57.0 || ^9.0.0), andeslint@^9.17.0satisfies this requirement. No action needed.src/ctx-embeddings.ts (1)
7-7: Structured logging implementation looks correct.The logger import and all three warning statements follow the structured format with
{ src: 'plugin:knowledge' }. Since these are utility functions without runtime context, omittingagentIdis appropriate.Also applies to: 285-285, 323-323, 607-607
src/service.ts (3)
45-48: Constructor simplification looks good.The constructor now only takes
runtimeparameter, which aligns with the AI summary indicatingKnowledgeConfigwas removed. Configuration is now handled viavalidateModelConfig(runtime)in thestartmethod.
51-156: Comprehensive structured logging in startup path.All logs consistently include
srcandagentId, with additional contextual fields (count, ctxEnabled, provider, model, agentName) where relevant. This will aid in debugging multi-agent scenarios.
875-884: Good audit trail for memory deletion.The deletion logging includes both
agentIdandmemoryId, which is useful for debugging and auditing. Note that errors fromruntime.deleteMemorywill propagate to the caller, which is appropriate.src/provider.ts (1)
70-76: Structured logging for RAG enrichment failures looks good.Both warning logs use a consistent format with granular source tag and include
agentId. Usingerror.messageinstead of the full error object is appropriate here since these are non-critical warnings.src/documents-provider.ts (1)
26-26: Structured logging implementation is consistent with other providers.Both log statements use the appropriate granular source tag
'plugin:knowledge:provider:documents'and includeagentIdfor traceability. The error handling gracefully logs and returns an empty result, which is appropriate for a provider.Also applies to: 122-122
src/index.ts (1)
81-86: Structured route/headless logging looks goodThe new debug/info calls with
{ src: 'plugin:knowledge' }keep behavior unchanged while standardizing format; no issues here.src/routes.ts (7)
96-213: Upload handler logging and error paths remain correctThe multipart and URL upload branches now emit structured logs with src/method/path/agentId/filename while preserving validation, cleanup, and error responses. No behavioral regressions spotted.
223-327: URL upload logging is consistent and informativeURL-based uploads now log key stages (missing agentId, fetch start, processing, and failures) with structured context including fileUrl and agentId. Control flow and response shapes are preserved.
411-444: Delete handler logging sequence is solidThe delete-by-ID flow logs the request, validates ID format, logs before/after delete, and logs failures, all with HTTP context and knowledgeId. Behavior and status codes are unchanged.
503-612: Frontend panel/asset logging is robustThe panel and asset handlers now emit structured logs for base path resolution, manifest usage, fallback asset choice, and errors. This should make debugging static asset issues easier without impacting responses.
745-865: Search logging is detailed and aligned with behaviorSearch handler logs now include agentId, clamped threshold/limit values, and result counts. The control flow (validation, embedding, searchMemories, enrichment) is unchanged and looks correct.
1007-1109: Graph node details logging correctly distinguishes roomId mismatchesThe node-details handler’s structured logs (documents/fragments counts, fallback without roomId, and mismatched roomId warnings) match the control flow and should aid debugging cross-room lookups without changing semantics.
1214-1229: Multer wrapper error logging is appropriateOn multer errors you now log
{ src: 'http', method, path, error: err.message }before returning a 400. That gives enough context without leaking file contents.src/docs-loader.ts (3)
11-29: Knowledge path logging is clear and non-disruptiveThe new warn/info logs around missing knowledge paths provide good guidance without changing the return value (
resolvedPath). No behavioral issues here.
34-124: Document loading logs are consistent and helpfulAll stages (missing path, no files, per-file skip/process/fail, and final summary) now emit structured logs with src/agentId/docsPath/filename/fragmentCount. Control flow and return structure remain the same.
129-150: Directory traversal error logging is safeThe
getAllFilescatch now logs dirPath anderror.messageunder src: 'plugin:knowledge'. It doesn’t change which files are returned, and failure to read a subdirectory still just yields a smaller list.src/document-processor.ts (10)
132-204: Fragment processing summary logging matches behaviorWarnings for empty text / no chunks, detailed rate-limit config logging, and final success/failure summaries all look correct and should greatly improve observability. Aside from the runtime argument noted above, no logic changes.
217-257: Text extraction logs are precise and safePDF vs non-PDF extraction is now logged with filename/contentType; UTF‑8 fallback and final error logs include
error.messagebut don’t expose raw content. Control flow and thrown errors are unchanged.
323-339: Chunk-splitting log aligns with existing behaviorYou log both token and derived char chunk sizes while still delegating to
splitChunks(documentText, tokenChunkSize, tokenChunkOverlap). That matches existing behavior; the extra char info is just diagnostic.
486-566: Embedding generation logs are well‑scopedErrors during embedding generation now log runtime.agentId, chunk index, and
error.message; the returned structures (success flag, index, text) are unchanged. Rate-limiter usage stays in the caller, so behavior is preserved.
575-619: CTX configuration and guidance logs are clear
getContextualizedChunksnow logs CTX enrichment configuration once per document and emits a targeted debug hint when CTX is disabled. This is a nice usability boost; behavior (fallback to plain chunks) is unchanged.
624-754: Context batch generation logging is detailed and matches control flowYou log provider/model, caching enablement, periodic progress, and errors per chunk. The new logs don’t alter prompt preparation or fallback behavior, and they integrate cleanly with the runtime-aware
validateModelConfig.
847-880: Zero‑vector handling logs are appropriate
generateEmbeddingWithValidationwarns once on zero vectors and returnssuccess: false, leaving higher-level logic to handle the failure. This matches the surrounding processing logic and avoids double-counting successes.
885-909: Rate‑limit retry logging is precise
withRateLimitRetrynow logs context, delay, and final failure with structured metadata. Control flow (single retry on 429) is unchanged and still safe.
914-975: Rate limiter logs are helpful and throttledYou only emit info logs for waits >5s and use debug for shorter waits, with clear reasons (
requestvstoken). The limiter behavior itself is unchanged.
981-1007: Knowledge generation summary logging is aligned with modeSummary and warning logs now include status, counts, and provider info under src: 'plugin:knowledge'. They only fire when
failedCount > 0or in development, so they shouldn’t spam production logs.src/config.ts (2)
16-96: Runtime‑aware config validation and CTX logging look correct
validateModelConfignow consistently prefersruntime.getSettingand falls back toprocess.env, and logs CTX/embedding configuration in a structured way. The subsequent call tovalidateConfigRequirements(config, assumePluginOpenAI, runtime)keeps previous validation behavior, just with richer logs.
213-216: Rate‑limit logging is consistent with new config modelThe “Rate limiting configuration” debug log includes agentId, provider, RPM/TPM, and max concurrent requests. With the helper fix above, it will accurately reflect both runtime and env inputs.
src/llm.ts (11)
20-39: Embedding entry point now correctly threads runtime context
generateTextEmbeddingnow validates config using the provided runtime and delegates to provider-specific helpers with structured error logging. Behavior is unchanged aside from embedding provider selection becoming runtime-aware.
47-114: Batch embedding flow and logs are sound
generateTextEmbeddingsBatchnow logs batch sizes and per-item failures with agentId context while still returning the same{ embedding, success, error?, index }structures. Control flow and backoff (simple 100ms between batches) are unchanged.
119-186: Provider‑specific embedding helpers integrate runtime cleanlyBoth OpenAI and Google embedding functions now log model name, dimensions/usage, and runtime.agentId. The API calls and return shapes are the same as before, so callers won’t see functional changes.
218-259: Runtime‑aware text generation dispatch looks correct
generateTextnow routes through Anthropic/OpenAI/OpenRouter/Google helpers withruntimefirst, preserving provider selection and error handling while adding structured error logs that include provider and modelName.
264-322: Anthropic retry/backoff logging is informativeAnthropic generation now logs per-attempt rate-limit retries with model, attempt, and delay, plus final token usage. Exponential backoff and maxRetries logic remain unchanged.
327-395: OpenAI/Google text generation logs are well‑scopedBoth helpers now log provider/model, total/input/output tokens with runtime.agentId while leaving request construction and return values untouched. This aligns with the structured logging goal.
415-489: OpenRouter text + caching: runtime threading and logs are correct
generateOpenRouterTextnow:
- Detects Claude/Gemini models,
- Optionally auto-extracts cached documents from
<document>tags while logging detection details,- Chooses between Claude/Gemini caching helpers and a standard path with a clear debug log.
All calls include runtime and preserve previous behavior; only logging and runtime propagation changed.
494-583: Claude caching helper integrates cache metrics logging
generateClaudeWithCachinglogs that explicit caching is used, then callslogCacheMetrics(runtime, result)and logs token usage under structured context. Message construction and providerOptions are unchanged.
588-650: Gemini caching helper logging is clearYou log whether implicit caching is likely, then feed everything through a simple, cache-friendly prompt format and call
logCacheMetrics. Behavior and return type remain the same.
655-685: Standard OpenRouter helper is unchanged aside from structured logsThe no-caching OpenRouter path still just calls
aiGenerateTextwith usage included, now logging provider/model/tokens under src: 'plugin:knowledge'.
690-697: Cache metrics logging correctly uses runtime context
logCacheMetricsnow takesruntimeand emits cacheTokens/cacheDiscount if present. It’s a pure logging helper and doesn’t affect control flow.
| if (!fs.existsSync(docsPath)) { | ||
| logger.warn(`Knowledge path does not exist: ${docsPath}`); | ||
| logger.warn({ src: 'plugin:knowledge', agentId: agentId, docsPath }, 'Knowledge path does not exist'); | ||
| return { total: 0, successful: 0, failed: 0 }; | ||
| } | ||
|
|
||
| logger.info(`Loading documents from: ${docsPath}`); | ||
| logger.info({ src: 'plugin:knowledge', agentId: agentId, docsPath }, 'Loading documents from path'); | ||
|
|
||
| // Get all files recursively | ||
| const files = getAllFiles(docsPath); | ||
|
|
||
| if (files.length === 0) { | ||
| logger.info('No files found in knowledge path'); | ||
| logger.info({ src: 'plugin:knowledge', agentId: agentId }, 'No files found in knowledge path'); | ||
| return { total: 0, successful: 0, failed: 0 }; | ||
| } | ||
|
|
||
| logger.info(`Found ${files.length} files to process`); | ||
| logger.info({ src: 'plugin:knowledge', agentId: agentId, filesCount: files.length }, 'Found files to process'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Resolve ESLint object-shorthand on logging objects
ESLint is complaining about non-shorthand properties like agentId: agentId and docsPath: docsPath in this file. Using shorthand will fix it:
- logger.info({ src: 'plugin:knowledge', agentId: agentId, docsPath }, 'Loading documents from path');
+ logger.info({ src: 'plugin:knowledge', agentId, docsPath }, 'Loading documents from path');Apply similar changes to the other logs noted in the static analysis hints.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!fs.existsSync(docsPath)) { | |
| logger.warn(`Knowledge path does not exist: ${docsPath}`); | |
| logger.warn({ src: 'plugin:knowledge', agentId: agentId, docsPath }, 'Knowledge path does not exist'); | |
| return { total: 0, successful: 0, failed: 0 }; | |
| } | |
| logger.info(`Loading documents from: ${docsPath}`); | |
| logger.info({ src: 'plugin:knowledge', agentId: agentId, docsPath }, 'Loading documents from path'); | |
| // Get all files recursively | |
| const files = getAllFiles(docsPath); | |
| if (files.length === 0) { | |
| logger.info('No files found in knowledge path'); | |
| logger.info({ src: 'plugin:knowledge', agentId: agentId }, 'No files found in knowledge path'); | |
| return { total: 0, successful: 0, failed: 0 }; | |
| } | |
| logger.info(`Found ${files.length} files to process`); | |
| logger.info({ src: 'plugin:knowledge', agentId: agentId, filesCount: files.length }, 'Found files to process'); | |
| if (!fs.existsSync(docsPath)) { | |
| logger.warn({ src: 'plugin:knowledge', agentId, docsPath }, 'Knowledge path does not exist'); | |
| return { total: 0, successful: 0, failed: 0 }; | |
| } | |
| logger.info({ src: 'plugin:knowledge', agentId, docsPath }, 'Loading documents from path'); | |
| // Get all files recursively | |
| const files = getAllFiles(docsPath); | |
| if (files.length === 0) { | |
| logger.info({ src: 'plugin:knowledge', agentId }, 'No files found in knowledge path'); | |
| return { total: 0, successful: 0, failed: 0 }; | |
| } | |
| logger.info({ src: 'plugin:knowledge', agentId, filesCount: files.length }, 'Found files to process'); |
🧰 Tools
🪛 ESLint
[error] 43-43: Expected property shorthand.
(object-shorthand)
[error] 47-47: Expected property shorthand.
(object-shorthand)
[error] 53-53: Expected property shorthand.
(object-shorthand)
[error] 57-57: Expected property shorthand.
(object-shorthand)
🤖 Prompt for AI Agents
In src/docs-loader.ts around lines 42 to 58, the logger calls use full object
property syntax (e.g., agentId: agentId, docsPath: docsPath) which trips ESLint
object-shorthand; change those to use property shorthand (agentId, docsPath,
filesCount) in each logger call in this block and apply the same shorthand
refactor to other logging objects referenced in the static analysis hints
elsewhere in the file.
| logger.debug( | ||
| `[Document Processor] 🔍 Filtered documents by URLs: ${fileUrls.length} URLs, found ${filteredMemories.length} matching documents` | ||
| ); | ||
| logger.debug({ src: 'http', method: req.method, path: req.path, agentId: agentId, urlCount: fileUrls.length, matchCount: filteredMemories.length }, 'Filtered documents by URLs'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix ESLint object-shorthand errors on logging contexts
ESLint is flagging several logging objects in this file (e.g., lines 390, 406, 465, 470, etc.) for not using property shorthand. For fields where the key matches the variable name (like agentId: agentId), you can switch to shorthand:
- logger.debug({ src: 'http', method: req.method, path: req.path, agentId: agentId, urlCount: fileUrls.length, matchCount: filteredMemories.length }, 'Filtered documents by URLs');
+ logger.debug({ src: 'http', method: req.method, path: req.path, agentId, urlCount: fileUrls.length, matchCount: filteredMemories.length }, 'Filtered documents by URLs');Apply the same pattern to other logs flagged in the static analysis hints to satisfy lint.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.debug({ src: 'http', method: req.method, path: req.path, agentId: agentId, urlCount: fileUrls.length, matchCount: filteredMemories.length }, 'Filtered documents by URLs'); | |
| logger.debug({ src: 'http', method: req.method, path: req.path, agentId, urlCount: fileUrls.length, matchCount: filteredMemories.length }, 'Filtered documents by URLs'); |
🧰 Tools
🪛 ESLint
[error] 390-390: Expected property shorthand.
(object-shorthand)
🤖 Prompt for AI Agents
In src/routes.ts around line 390 (and similarly at lines 406, 465, 470, etc.),
the logging context objects use explicit key: value pairs where the key equals
the variable name (e.g., agentId: agentId), which triggers ESLint's
object-shorthand rule; update those logger calls to use property shorthand for
any fields whose key matches the variable name (leave explicit mappings only
when the property name must differ), apply the same change to all other logging
objects flagged by static analysis in this file, and re-run the linter to
confirm no object-shorthand errors remain.
Prior merge of: elizaOS/eliza#6169
Summary
@elizaos/structured-loggingrulelint:checkscript for CITest plan
Summary by CodeRabbit
Release Notes
New Features
getMemories,countMemories, anddeleteMemoryfor flexible memory retrieval, counting, and deletion.Chores
✏️ Tip: You can customize this high-level summary in your review settings.