Conversation
WalkthroughAdds an ESLint config and lint devDependencies, exposes four new model-related config options, converts many logs to structured runtime.logger calls with contextual metadata, updates several helper and image-storage function signatures to accept a runtime context, and normalizes string quotes across the codebase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip ✨ Issue Enrichment is now available for GitHub issues!CodeRabbit can now help you manage issues more effectively:
Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseThanks 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: 0
🧹 Nitpick comments (2)
package.json (1)
3-3: Lint tooling and config metadata look solid; consider CI integrationThe version bump, new devDependencies, and
lint/lint:checkscripts are consistent with the new shared ESLint config. The additional IMAGE/EMBEDDING-relatedagentConfigentries are coherent and well documented. As a follow‑up, it’d be good to wirelint:checkinto CI if it isn’t already, so structured-logging rules are enforced automatically.Also applies to: 35-45, 49-50, 116-178
src/init.ts (1)
32-45: Consistent structured logging for validation and error pathsThe validation success/failure, fetch error, and outer configuration error branches now all emit structured logs with
src,agentId, and relevant details (status,statusText,error). This should make diagnosing OpenRouter configuration issues much easier. If you touch this again, you might consider factoring a small helper for the repeated{ src: "plugin:openrouter", agentId: runtime.agentId }context, but it’s not required.Also applies to: 51-57, 66-68
📜 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 (14)
eslint.config.js(1 hunks)package.json(2 hunks)src/index.ts(3 hunks)src/init.ts(3 hunks)src/models/embedding.ts(4 hunks)src/models/image.ts(6 hunks)src/models/index.ts(1 hunks)src/models/object.ts(1 hunks)src/models/text.ts(4 hunks)src/types/index.ts(1 hunks)src/utils/config.ts(11 hunks)src/utils/events.ts(1 hunks)src/utils/helpers.ts(6 hunks)src/utils/image-storage.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/models/text.ts (1)
src/utils/helpers.ts (1)
decodeBase64Fields(180-210)
src/index.ts (3)
src/models/text.ts (1)
handleTextLarge(170-178)src/models/object.ts (2)
handleObjectSmall(65-70)handleObjectLarge(75-80)src/models/image.ts (2)
handleImageDescription(23-88)handleImageGeneration(93-220)
src/models/embedding.ts (2)
src/models/index.ts (1)
handleTextEmbedding(4-4)src/utils/config.ts (4)
getEmbeddingModel(117-123)getSetting(10-16)getApiKey(47-49)getBaseURL(23-39)
🔇 Additional comments (13)
src/utils/events.ts (1)
25-30: Numeric coercion remains safe and clearToken coercion still handles missing values and defaults cleanly; this is a no-op refactor with consistent style.
eslint.config.js (1)
1-3: ESLint config wiring looks correctRe-exporting the shared
pluginConfigkeeps linting consistent with the monorepo and is idiomatic for flat configs.src/types/index.ts (1)
28-28: Type literal change is purely stylisticSwitching to double quotes for
"tool-call"keeps the literal identical at the type level; no behavior or typing impact.src/init.ts (1)
20-23: Missing-key log now structured and clearThe warning for absent
OPENROUTER_API_KEYcleanly addssrcandagentIdcontext while keeping the message concise; no behavior change.src/models/object.ts (1)
32-40: Structured model-selection log is a nice improvementLogging
{ src: "plugin:openrouter:model:object", agentId, modelType, model }at debug level before generation gives clear, structured insight into which model is used without changing behavior. This fits well with the standardized logging approach.src/models/index.ts (1)
1-4: Exports remain unchanged; style is consistentRe-exports for the model handlers are intact; only quote style is adjusted for consistency.
src/utils/image-storage.ts (1)
104-105: Image storage logs now structured and consistentThe success and failure logs for saving and deleting images now include
srcandfilepath(anderrorwhere relevant) as structured fields, with debug for normal operations and warn for failures. This aligns nicely with the standardized logging pattern across the plugin.Also applies to: 122-134
src/models/image.ts (1)
30-37: LGTM! Consistent structured logging implementation.The structured logging changes are well-implemented with consistent metadata fields (src, agentId) and relevant context for each log event. The messages are concise and descriptive.
Also applies to: 75-82, 98-105, 157-164, 174-181, 199-206, 210-217
src/models/text.ts (1)
43-51: LGTM! Well-structured logging for text generation.The structured logging changes provide good context for debugging text generation and tool execution. The metadata fields are consistent with the project-wide pattern.
Also applies to: 71-78
src/models/embedding.ts (1)
26-40: LGTM! Comprehensive structured logging for embedding workflow.The structured logging changes provide excellent diagnostic coverage for all embedding workflow paths, including error cases, edge cases, and success scenarios. The metadata fields consistently include src and agentId, with relevant additional context for each situation.
Also applies to: 42-54, 62-65, 71-79, 82-88, 107-120, 127-133, 137-150, 162-170, 172-182
src/index.ts (1)
39-42: LGTM! New configuration options properly integrated.The new config options (IMAGE_MODEL, IMAGE_GENERATION_MODEL, EMBEDDING_MODEL, EMBEDDING_DIMENSIONS) are correctly added and properly used as fallback values in the config utility functions.
src/utils/config.ts (1)
1-157: LGTM! Consistent quote style formatting.All changes are purely cosmetic, standardizing string literals to use double quotes throughout the file. No functional changes.
src/utils/helpers.ts (1)
23-26: LGTM! Comprehensive structured logging for helper utilities.The structured logging changes provide excellent diagnostic coverage across all helper functions, including JSON repair, response structure analysis, tool execution fallbacks, and base64 decoding. The metadata and messages are well-chosen for debugging.
Also applies to: 52-64, 71-75, 92-95, 115-118, 128-131, 138-141, 146-146, 151-154, 192-195, 197-200
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
eslint.config.js(1 hunks)
🔇 Additional comments (2)
eslint.config.js (2)
10-10: LGTM!The re-export pattern is clean and aligns with the PR objective of adopting standardized ElizaOS configuration.
1-1: The import path is valid and correctly references the @elizaos/config package.The import statement
import pluginConfig from "@elizaos/config/eslint/eslint.config.plugin.js";matches the documented export from the@elizaos/configpackage (version ^1.6.5). This is the standard way to import the ESLint plugin configuration from this package, as confirmed by package documentation.
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)
src/utils/helpers.ts (1)
184-214: Critical: Missing runtime parameter in recursive calls.The
decodeBase64Fieldssignature was updated to acceptruntimeas the first parameter, but the recursive calls on lines 189 and 208 don't pass theruntimeparameter. This will cause runtime errors.Apply this diff:
if (Array.isArray(obj)) - return obj.map((item) => decodeBase64Fields(item, depth + 1)); + return obj.map((item) => decodeBase64Fields(runtime, item, depth + 1));} else if (value && typeof value === "object") { - decoded[key] = decodeBase64Fields(value, depth + 1); + decoded[key] = decodeBase64Fields(runtime, value, depth + 1); } else {
🧹 Nitpick comments (3)
src/utils/image-storage.ts (1)
105-135: Inconsistent quote style for src field.This file uses single quotes (
'plugin:openrouter') whilesrc/init.tsuses double quotes ("plugin:openrouter"). Consider standardizing to match the rest of the codebase.Apply this diff to use double quotes consistently:
- runtime.logger.debug({ src: 'plugin:openrouter', agentId: runtime.agentId, filepath }, "Saved generated image"); + runtime.logger.debug({ src: "plugin:openrouter", agentId: runtime.agentId, filepath }, "Saved generated image");- runtime.logger.debug({ src: 'plugin:openrouter', agentId: runtime.agentId, filepath }, "Deleted image"); + runtime.logger.debug({ src: "plugin:openrouter", agentId: runtime.agentId, filepath }, "Deleted image");- runtime.logger.warn( - { src: 'plugin:openrouter', agentId: runtime.agentId, filepath, error: String(error) }, - "Failed to delete image", - ); + runtime.logger.warn( + { src: "plugin:openrouter", agentId: runtime.agentId, filepath, error: String(error) }, + "Failed to delete image", + );- runtime.logger.warn( - { src: 'plugin:openrouter', agentId: runtime.agentId, filepath, error: String(error) }, - "Failed to delete image", - ); + runtime.logger.warn( + { src: "plugin:openrouter", agentId: runtime.agentId, filepath, error: String(error) }, + "Failed to delete image", + );src/utils/helpers.ts (2)
146-146: Add braces for consistency.ESLint is flagging the missing braces after the
ifcondition. While this is valid JavaScript, adding braces improves consistency and prevents potential bugs if the code is modified later.Apply this diff:
- if (repairParseError instanceof Error) throw repairParseError; + if (repairParseError instanceof Error) { + throw repairParseError; + }
159-159: Add braces for consistency.Same issue as line 146 - add braces after the
ifcondition.Apply this diff:
- if (error instanceof Error) throw error; + if (error instanceof Error) { + throw error; + }
📜 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 (7)
package.json(2 hunks)src/init.ts(4 hunks)src/models/image.ts(7 hunks)src/models/object.ts(2 hunks)src/models/text.ts(5 hunks)src/utils/helpers.ts(9 hunks)src/utils/image-storage.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/models/text.ts
- src/models/image.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/models/object.ts (2)
src/utils/helpers.ts (3)
getJsonRepairFunction(8-30)emitModelUsageEvent(33-33)handleObjectGenerationError(114-162)src/utils/events.ts (1)
emitModelUsageEvent(11-44)
🪛 ESLint
src/utils/helpers.ts
[error] 146-146: Expected { after 'if' condition.
(curly)
🔇 Additional comments (7)
src/init.ts (1)
20-68: LGTM! Structured logging implemented consistently.All logging statements have been successfully converted to structured format with appropriate context (src, agentId) and additional fields where relevant. The log levels and messages are appropriate for each scenario.
src/utils/image-storage.ts (2)
51-56: Breaking change: Runtime parameter added to public API.The function signature now requires
runtimeas the first parameter to enable structured logging. Ensure all callers have been updated accordingly.
114-114: Breaking change: Runtime parameter added.Consistent with the broader refactor. Ensure callers have been updated.
src/models/object.ts (1)
50-58: LGTM! Helper function calls updated correctly.Both
getJsonRepairFunctionandhandleObjectGenerationErrornow correctly receive theruntimeparameter, aligning with their updated signatures.src/utils/helpers.ts (3)
8-30: LGTM! Function signature and logging updated correctly.The
runtimeparameter enables structured logging. Note the single-quote style for'plugin:openrouter'is inconsistent withsrc/init.ts(which uses double quotes), but this is a minor style issue.
38-67: LGTM! Response logging refactored to structured format.The function now accepts
runtimeand uses structured logging with comprehensive context fields for debugging.
72-78: LGTM! Tool response handler updated.Function signature and logging correctly updated to use structured format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/models/object.ts (1)
3-3: Remove unused logger import.The global
loggerimport is no longer used since the code now usesruntime.loggerfor structured logging. This import should be removed to keep the code clean.Apply this diff:
import { ModelType, - logger, type IAgentRuntime, type ObjectGenerationParams, } from "@elizaos/core";
📜 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 (2)
eslint.config.js(1 hunks)src/models/object.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/models/object.ts (2)
src/utils/helpers.ts (3)
getJsonRepairFunction(8-30)emitModelUsageEvent(33-33)handleObjectGenerationError(114-162)src/utils/events.ts (1)
emitModelUsageEvent(11-44)
🔇 Additional comments (4)
eslint.config.js (1)
1-10: ESLint wrapper config looks correct and consistent with monorepo setupImport/export wiring is straightforward, the header comment now correctly names
plugin-openrouter, and delegating to the shared plugin config keeps rules centralized. No changes needed.src/models/object.ts (3)
32-40: Well done! Structured logging correctly implemented.The logging now uses
runtime.logger.debugwith appropriate structured metadata (src, agentId, modelType, model). This aligns perfectly with the PR objectives and addresses the previous review feedback.
50-50: Correct update to helper function signature.The call to
getJsonRepairFunction(runtime)correctly matches the updated signature insrc/utils/helpers.ts, which now requires a runtime parameter for structured logging.
58-58: Correct update to error handler signature.The call to
handleObjectGenerationError(runtime, error)correctly matches the updated signature insrc/utils/helpers.ts, which now requires runtime as the first parameter for structured error logging.
2 participants
Prior merge of: elizaOS/eliza#6169
Summary
@elizaos/structured-loggingrulelint:checkscript for CITest plan
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.