Conversation
Eliminates legacy configuration utilities and their exports to streamline the config resolution logic. Updates tests to use the recommended config accessor, improving maintainability and reducing deprecated code paths.
WalkthroughThis update introduces a comprehensive refactor and extension of the configuration, graph, and logging infrastructure for the Flow Service. It modularizes configuration handling using Quadstore-backed graphs, introduces new utility and factory modules for RDF and JSON-LD operations, updates configuration types and defaults, and transitions service and route logic to use a singleton asynchronous configuration accessor. The update also adds new ESLint and formatting rules, test cases, and documentation improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI/Env/File as Config Sources
participant Loader as ServiceConfigLoader
participant Quadstore as Quadstore (Graphs)
participant Accessor as ServiceConfigAccessor
participant App as Flow Service
CLI/Env/File->>Loader: Provide config input (CLI, env, file)
Loader->>Quadstore: Load/merge configs into named graphs
Loader->>Quadstore: Merge input and platform defaults
App->>Accessor: Request config values (getHost, getPort, getMeshPaths)
Accessor->>Quadstore: Query merged config graph (SPARQL)
Quadstore-->>Accessor: Return config values
Accessor-->>App: Return config values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (23)
flow-service/tests/integration/logging/log-retention-config.test.ts (1)
7-8: Relative-path creep – consider an import mapBumping the path up another
..level fixes the immediate issue, but the test tree keeps getting deeper and these paths become fragile.
Deno supportsimport_map.json; mapping@src/*→./src/*would let youimport { … } from "@src/config/index.ts"and avoid future churn.flow-service/tests/integration/logging/entry-init.test.ts (1)
7-8: Use an import alias to avoid brittle relative pathsSame remark as in the previous test file: consider switching to an import map (or path alias via a build step) so these deep
../../../segments don’t break when the directory layout changes again.flow-service/tests/integration/logging/sentry-logging.test.ts (1)
7-8: Repeated deep relative importYet another deep-relative import; an
import_map.jsonentry (e.g.@utils/logger) would simplify all three logging tests and improve readability.flow-service/tests/integration/logging/logger.test.ts (1)
7-7: Minor: centralise logger importFor consistency with the other suggestions, pulling
loggerin via an import-map alias keeps this single-line change from needing edits every time the folder depth changes.flow-service/src/config/resolution/service-config-validator.ts (1)
16-20: Consider edge case: port value 0.The validation correctly checks the valid port range (1-65535), but the condition
!portwill also reject port 0, which is technically correct since port 0 is reserved. However, for clarity, consider being more explicit:- if (!port || port <= 0 || port > 65535) { + if (typeof port !== 'number' || port <= 0 || port > 65535) {This makes the validation more explicit about type checking.
flow-service/tests/integration/quadstore.test.ts (1)
12-12: Consider extracting test bundle setup to a helper.Since this pattern might be reused in other integration tests, consider extracting the quadstore bundle setup to a test utility:
// In a test-utils file export function createTestQuadstoreBundle() { const backend = new MemoryLevel(); const df = new DataFactory(); const store = new Quadstore({ backend, dataFactory: df }); return { store, df, backend }; }flow-core/src/platform-constants.ts (1)
12-19: Consider using type predicate for better type safety.The function could be enhanced with a type predicate to provide better TypeScript inference:
-export function validateLogLevel(level: unknown): LogLevel { +export function validateLogLevel(level: unknown): level is LogLevel { if (typeof level !== 'string' || !validLogLevels.includes(level as LogLevel)) { throw new Error( `Invalid log level: ${level}. Must be one of: ${validLogLevels.join(', ')}`, ); } - return level as LogLevel; + return true; +} + +export function getValidatedLogLevel(level: unknown): LogLevel { + if (!validateLogLevel(level)) { + throw new Error( + `Invalid log level: ${level}. Must be one of: ${validLogLevels.join(', ')}`, + ); + } + return level;However, the current implementation is simpler and adequate for the use case.
flow-core/src/types.ts (1)
5-5: Inconsistent import style.The
Engineimport uses a different style than the other imports. Consider consistency:-import { Engine } from 'npm:quadstore-comunica'; +import type { Engine } from 'npm:quadstore-comunica';Since it's only used as a type in the interface.
flow-service/src/quadstoreDefaultBundle.ts (1)
3-6: Consider error handling for initialization.While the current implementation is clean, consider adding error handling for quadstore initialization, especially in production environments:
+try { const backend = new MemoryLevel(); const df = new DataFactory(); const store = new Quadstore({ backend, dataFactory: df }); const engine = new Engine(store); + await store.open(); +} catch (error) { + console.error('Failed to initialize default quadstore bundle:', error); + throw error; +}flow-core/src/utils/rdfjs-utils.ts (2)
25-25: Consider type safety for values array.The type assertion
as any[]bypasses TypeScript's type checking. Consider using proper type guards or validation.- for (const value of values as any[]) { + const valuesArray = Array.isArray(values) ? values : [values]; + for (const value of valuesArray) {
34-37: Document the JSON serialization fallback behavior.The fallback to JSON.stringify() for complex objects should be documented as it may not preserve semantic meaning in RDF contexts.
} else { - // For complex nested objects, serialize as JSON string literal + // For complex nested objects, serialize as JSON string literal + // Note: This loses semantic RDF meaning and should be used sparingly object = df.literal(JSON.stringify(value)); }flow-core/tests/unit/quadstore-utils.test.ts (1)
5-21: Consider testing error conditions.The tests only cover happy path scenarios. Consider adding tests for error conditions such as invalid graph URIs or quadstore failures.
Example error test:
Deno.test('countQuadsInStream handles empty streams', async () => { const { store } = defaultQuadstoreBundle; const emptyGraph = df.namedNode('urn:emptyGraph'); const stream = store.match(undefined, undefined, undefined, emptyGraph); const count = await countQuadsInStream(stream); assertEquals(count, 0); });flow-service/tests/integration/logging/sentry-init.test.ts (2)
15-16: Consider test isolation for environment variables.Setting environment variables globally could affect other tests. Consider using Deno's test isolation or saving/restoring original values.
+ // Save original env values for restoration + const originalSentryEnabled = Deno.env.get('FLOW_SENTRY_ENABLED'); + const originalSentryDsn = Deno.env.get('FLOW_SENTRY_DSN'); + try { // Set environment variables for Sentry Deno.env.set('FLOW_SENTRY_ENABLED', 'true'); + + // ... rest of test logic + } finally { + // Restore original environment + if (originalSentryEnabled !== undefined) { + Deno.env.set('FLOW_SENTRY_ENABLED', originalSentryEnabled); + } else { + Deno.env.delete('FLOW_SENTRY_ENABLED'); + } + // Similar for FLOW_SENTRY_DSN + }
18-23: Add more specific error handling for missing DSN.The error message could be more helpful by providing guidance on how to set up the DSN.
if (!sentryDsn) { throw new Error( - 'FLOW_SENTRY_DSN not found in environment. Please check your .env file.', + 'FLOW_SENTRY_DSN not found in environment. Please add FLOW_SENTRY_DSN=your_dsn_here to your .env file. Get your DSN from https://sentry.io/settings/projects/', ); }flow-core/src/deps.ts (1)
20-21: Consider simplifying the jsonld import pattern.The pattern of importing as default then re-exporting with a name is unusual. Consider using a direct named import if the library supports it.
-import jsonldDefault from "npm:jsonld"; -export const jsonld = jsonldDefault; +export { default as jsonld } from "npm:jsonld";Or if you need the default behavior:
-import jsonldDefault from "npm:jsonld"; -export const jsonld = jsonldDefault; +export { default as jsonld } from "npm:jsonld";flow-service/src/config/resolution/service-config-utils.ts (2)
31-38: Consider return type precision for mergeConfigContext.The return type
typeof PLATFORM_SERVICE_DEFAULTSmight be too broad since the merged result could have additional properties from inputOptions.export function mergeConfigContext( context: ServiceConfigContext, -): typeof PLATFORM_SERVICE_DEFAULTS { +): typeof PLATFORM_SERVICE_DEFAULTS & Partial<ServiceConfigInput> { return mergeConfigs( context.defaultOptions, context.inputOptions as Partial<typeof PLATFORM_SERVICE_DEFAULTS>, ); }
5-12: Enhance function documentation with examples.The documentation could benefit from usage examples to clarify the intended use patterns.
/** * Retrieves a configuration value from the context, returning the value from input options if defined, or falling back to the default options. + * + * @example + * ```typescript + * const logLevel = getConfigValue<string>(context, 'logLevel', 'defaultLogLevel'); + * ``` * * @param context - The service configuration context containing both input and default options * @param inputKey - The key to look up in the input options * @param defaultKey - The key to look up in the default options if the input value is undefined * @returns The resolved configuration value */flow-service/src/config/resolution/service-config-loader.ts (1)
64-73: Consider adding validation for the nested logging configuration structure.While
validateLogLevelensures the log level is valid, there's no validation that the nested JSON-LD structure for logging configuration is properly formed. Missing required fields like@typecould cause issues downstream.Consider adding validation for the complete logging configuration structure:
if (cliOptions.logLevel !== undefined) { + const validatedLogLevel = validateLogLevel(cliOptions.logLevel); config['fsvc:hasLoggingConfig'] = { '@type': 'fsvc:LoggingConfig', 'fsvc:hasConsoleChannel': { '@type': 'fsvc:LogChannelConfig', 'fsvc:logChannelEnabled': true, - 'fsvc:logLevel': validateLogLevel(cliOptions.logLevel), + 'fsvc:logLevel': validatedLogLevel, }, }; }flow-service/src/config/loaders/quadstore-loader.ts (1)
59-65: Potential performance issue with sequential quad checking.The merging logic iterates through all platform default quads and checks existence individually (Line 61), which could be inefficient for large configuration datasets.
Consider batch operations or indexing for better performance:
// Copy platform service defaults quads if not overridden const platformQuads = store.match(undefined, undefined, undefined, df.namedNode(GRAPH_NAMES.platformServiceDefaults)); + const mergedSubjects = new Set(); + + // First, collect all subjects that already exist in merged graph + const existingQuads = store.match(undefined, undefined, undefined, df.namedNode(GRAPH_NAMES.mergedServiceConfig)); + for await (const q of existingQuads as any) { + mergedSubjects.add(q.subject.value + '|' + q.predicate.value); + } + for await (const q of platformQuads as any) { - const exists = await store.countQuads(q.subject, q.predicate, undefined, df.namedNode(GRAPH_NAMES.mergedServiceConfig)); - if (exists === 0) { + const key = q.subject.value + '|' + q.predicate.value; + if (!mergedSubjects.has(key)) { await store.put(df.quad(q.subject, q.predicate, q.object, df.namedNode(GRAPH_NAMES.mergedServiceConfig))); } }flow-service/src/config/defaults.ts (1)
21-21: Address the TODO comment.The TODO comment suggests renaming
PLATFORM_NODE_DEFAULTSto better indicate it's for root MeshNodes. This should be tracked and resolved.The TODO indicates
PLATFORM_NODE_DEFAULTSshould be renamed to better represent root MeshNodes. Would you like me to generate a refactoring plan or open an issue to track this task?flow-service/src/config/loaders/jsonld-loader.ts (1)
274-282: Validation function simplified but may be too permissive.The new
validateJsonLdfunction only checks for@typeand@contextproperties but doesn't validate their values or structure. This might allow invalid configurations to pass validation.Consider more comprehensive validation:
export function validateJsonLd(inputJsonLd: NodeObject): void { if (!inputJsonLd['@type']) { throw new ConfigError('Configuration must have an "@type" property'); } if (!inputJsonLd['@context']) { throw new ConfigError('Configuration must have an "@context" property'); } + // Validate @type is a string + if (typeof inputJsonLd['@type'] !== 'string') { + throw new ConfigError('Configuration "@type" must be a string'); + } + // Validate @context is an object or string + if (typeof inputJsonLd['@context'] !== 'object' && typeof inputJsonLd['@context'] !== 'string') { + throw new ConfigError('Configuration "@context" must be an object or string'); + } }flow-service/src/config/config-types.ts (1)
88-91: Address the TODO comment for AttributedTo interface.The
AttributedTointerface has an incomplete definition with a TODO comment.Would you like me to help define the complete structure for the
AttributedTointerface or create an issue to track this task?flow-core/src/utils/quadstore-utils.ts (1)
35-50: Optimize stream usage and address type assertion.The function creates two separate streams for the same data. Consider using a single stream to improve performance. Also, the
as anytype assertion suggests a type mismatch.export async function clearGraph( graph: RDF.NamedNode, { store }: { store: Quadstore } = defaultQuadstoreBundle ): Promise<number> { - const matchStream = store.match(undefined, undefined, undefined, graph); - const count = await countQuadsInStream(matchStream); - - if (count > 0) { - const delStream = store.match(undefined, undefined, undefined, graph); - await store.delStream(delStream as any); - } - - return count; + const matchStream = store.match(undefined, undefined, undefined, graph); + const quads: RDF.Quad[] = []; + + for await (const quad of matchStream) { + quads.push(quad); + } + + if (quads.length > 0) { + await store.multiDel(quads); + } + + return quads.length; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
deno.lockis excluded by!**/*.lockflow-service/deno.lockis excluded by!**/*.lockflow-service/logs/test-pretty-direct.logis excluded by!**/*.log
📒 Files selected for processing (29)
flow-core/src/deps.ts(1 hunks)flow-core/src/platform-constants.ts(1 hunks)flow-core/src/types.ts(1 hunks)flow-core/src/utils/quadstore-utils.ts(1 hunks)flow-core/src/utils/rdfjs-utils.ts(1 hunks)flow-core/tests/unit/quadstore-utils.test.ts(1 hunks)flow-service/deno.json(1 hunks)flow-service/src/config/config-types.ts(6 hunks)flow-service/src/config/defaults.ts(5 hunks)flow-service/src/config/index.ts(2 hunks)flow-service/src/config/loaders/env-loader.ts(2 hunks)flow-service/src/config/loaders/jsonld-loader.ts(7 hunks)flow-service/src/config/loaders/quadstore-loader.ts(1 hunks)flow-service/src/config/node-config.default.jsonld(1 hunks)flow-service/src/config/resolution/service-config-accessor.ts(1 hunks)flow-service/src/config/resolution/service-config-loader.ts(1 hunks)flow-service/src/config/resolution/service-config-resolver.ts(2 hunks)flow-service/src/config/resolution/service-config-utils.ts(1 hunks)flow-service/src/config/resolution/service-config-validator.ts(1 hunks)flow-service/src/quadstoreDefaultBundle.ts(1 hunks)flow-service/tests/integration/logging/entry-init.test.ts(1 hunks)flow-service/tests/integration/logging/formatting-direct.test.ts(3 hunks)flow-service/tests/integration/logging/handle-caught-error.test.ts(1 hunks)flow-service/tests/integration/logging/log-retention-config.test.ts(1 hunks)flow-service/tests/integration/logging/logger.test.ts(1 hunks)flow-service/tests/integration/logging/pretty-format-simple.test.ts(1 hunks)flow-service/tests/integration/logging/sentry-init.test.ts(1 hunks)flow-service/tests/integration/logging/sentry-logging.test.ts(1 hunks)flow-service/tests/integration/quadstore.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
flow-service/src/config/loaders/env-loader.ts (1)
flow-service/src/config/types.ts (6)
EnvironmentConfig(154-174)NodeConfigInput(119-130)NodeConfig(61-72)ServiceConfig(75-85)ServiceConfigInput(107-117)getConsoleLogLevel(217-220)
flow-service/tests/integration/logging/handle-caught-error.test.ts (1)
flow-service/src/config/types.ts (4)
ConfigError(177-182)ConfigValidationError(184-189)constructor(185-188)constructor(178-181)
flow-service/tests/integration/quadstore.test.ts (2)
flow-core/src/deps.ts (4)
MemoryLevel(18-18)DataFactory(17-17)Quadstore(16-16)assertEquals(3-3)flow-core/src/utils/quadstore-utils.ts (3)
clearGraph(35-50)copyGraph(54-71)countQuadsInStream(8-15)
flow-service/src/config/resolution/service-config-validator.ts (3)
flow-service/src/config/config-types.ts (2)
ServiceConfigContext(134-137)ConfigError(179-184)flow-service/src/config/resolution/service-config-accessor.ts (2)
port(10-12)sentryDsn(57-62)flow-service/src/config/resolution/service-config-utils.ts (1)
getConfigValue(13-22)
flow-core/src/platform-constants.ts (2)
flow-service/src/config/index.ts (1)
LogLevel(14-14)flow-service/src/config/types.ts (3)
LogChannelConfig(24-36)LoggingConfig(38-43)getConsoleLogLevel(217-220)
flow-service/src/quadstoreDefaultBundle.ts (1)
flow-core/src/deps.ts (4)
MemoryLevel(18-18)DataFactory(17-17)Quadstore(16-16)Engine(23-23)
flow-service/src/config/resolution/service-config-accessor.ts (3)
flow-service/src/config/config-types.ts (3)
ServiceConfigContext(134-137)DelegationChain(102-105)AttributedTo(88-91)flow-service/src/config/resolution/service-config-utils.ts (1)
getConfigValue(13-22)flow-service/src/config/types.ts (4)
ServiceConfig(75-85)ServiceConfigInput(107-117)ServiceConfigContext(133-136)ContainedServicesConfig(46-53)
flow-service/src/config/resolution/service-config-resolver.ts (1)
flow-service/src/config/types.ts (3)
ServiceConfigContext(133-136)ServiceConfigInput(107-117)ServiceConfig(75-85)
flow-core/src/types.ts (2)
flow-core/src/deps.ts (3)
Quadstore(16-16)DataFactory(17-17)Engine(23-23)flow-service/src/config/types.ts (1)
NodeConfigInput(119-130)
flow-service/src/config/config-types.ts (2)
flow-core/src/deps.ts (2)
ContextDefinition(31-31)NodeObject(29-29)flow-service/src/config/types.ts (9)
ServiceConfig(75-85)NodeConfigInput(119-130)NodeConfig(61-72)ServiceConfigInput(107-117)NodeConfigContext(138-141)JSONLDContext(9-15)ServiceConfigContext(133-136)JSONLDBase(17-21)ContainedServicesConfig(46-53)
flow-core/src/utils/quadstore-utils.ts (3)
flow-service/src/quadstoreDefaultBundle.ts (1)
defaultQuadstoreBundle(10-10)flow-core/src/utils/rdfjs-utils.ts (1)
jsonldToQuads(9-47)flow-core/src/types.ts (1)
QuadstoreBundle(8-13)
🔇 Additional comments (33)
flow-service/src/config/node-config.default.jsonld (1)
9-9: Check for dangling references to the old distribution IRIChanging the identifier to
conf:defaultMeshNodeConfigis fine, but make sure every place that previously referencedconf:defaultNodeConfig(tests, docs, code that does IRI comparisons, etc.) has been updated.
A quickgrepacross the repo is usually enough to catch stragglers.flow-service/tests/integration/logging/pretty-format-simple.test.ts (1)
11-11: Import path verified
The import../../../src/utils/logger.tsinpretty-format-simple.test.tscorrectly resolves toflow-service/src/utils/logger.ts. No changes needed.flow-service/deno.json (1)
11-14: LGTM! Necessary permission update for enhanced logging.The addition of
./logsto the--allow-writepermissions across all tasks is appropriate and necessary to support the enhanced logging infrastructure introduced in this PR. This change enables the Deno runtime to write log files to the designated logs directory.flow-service/tests/integration/logging/handle-caught-error.test.ts (3)
9-13: LGTM! Import path corrections look appropriate.The import paths have been updated to reflect the new directory structure, changing from
../../to../../../for error classes.
14-14: Good refactoring: ConfigValidationError moved to config-types.ts.The import change from
config/types.tstoconfig/config-types.tsindicates a good reorganization of configuration types, which aligns with the configuration system refactoring objectives of this PR.
8-8: Import path for handleCaughtError verifiedThe
handleCaughtErrorfunction is correctly exported fromflow-service/src/utils/logger.ts, and the test import path (../../../src/utils/logger.ts) resolves without issue. No further changes needed.flow-service/tests/integration/logging/formatting-direct.test.ts (2)
15-22: Nice formatting improvement for LogContext interface.The union types have been reformatted to a more compact style, improving readability while maintaining the same type semantics.
74-74: Minor formatting improvement in constructor.Added space inside the braces for better code formatting consistency.
flow-service/src/config/loaders/env-loader.ts (2)
227-227: Appropriate type rename: NodeConfig → MeshNodeConfig.The change from
'conf:NodeConfig'to'conf:MeshNodeConfig'better reflects the purpose and scope of these configuration objects. This aligns with the broader configuration system improvements in this PR.
11-13: Approve configuration refactoringVerified that:
flow-service/src/config/config-types.tsexportsEnvironmentConfig,ServiceConfigInput, andConfigErrorflow-core/src/platform-constants.tsexportsvalidateLogLevelThese changes cleanly centralize validation logic and reorganize configuration types as intended. Great work—no further action required.
flow-service/src/config/resolution/service-config-validator.ts (1)
12-40: Strong validation logic with proper separation of concerns.The validation function correctly handles the essential service configuration parameters with appropriate error messages. The modularization improves maintainability by separating validation from resolution logic.
flow-service/tests/integration/quadstore.test.ts (2)
6-50: Comprehensive integration test with good coverage.The test effectively validates the core quadstore graph manipulation utilities with a realistic setup. The test sequence covers the key operations and verifies state at each step.
52-55: Good helper function implementation.The
countQuadsInGraphhelper function is well-implemented and provides a clean interface for counting quads in a specific graph.flow-core/src/platform-constants.ts (1)
1-19: Excellent centralization of log level validation logic.This module effectively consolidates log level validation that was previously scattered across the codebase. The use of
as constwith proper TypeScript union type derivation provides both compile-time and runtime type safety.flow-core/src/types.ts (1)
8-13: Well-designed interface with appropriate generics.The
QuadstoreBundleinterface effectively groups related quadstore components with proper type safety. The generic constraint and optional engine property provide good flexibility while maintaining type safety.flow-service/src/quadstoreDefaultBundle.ts (2)
1-11: Solid singleton implementation with proper initialization.The default quadstore bundle provides a well-configured in-memory singleton that aligns with the PR objectives. The initialization sequence is correct and the export pattern enables reuse across the application.
8-8: No action needed: Deno supports top-level await by default
Both your root and flow-service deno.json configs don’t override the default TStargetormodule(which are ESNext), so top-level await is fully supported in your environment.flow-core/src/deps.ts (1)
27-35: Selective JSON-LD type exports are sufficient as-is.A codewide search for additional JSON-LD types (e.g.
CompactedDocument,Frame) turned up no usages, so the current exports (JsonLdDocument,NodeObject,ValueObject,ContextDefinition) cover all existing code paths.flow-service/src/config/resolution/service-config-loader.ts (1)
13-39: LGTM: Well-structured cascading configuration loader.The function correctly implements a cascading configuration pattern, loading from environment variables, config file, and CLI options in proper precedence order. The async handling of the config file loading is appropriate.
flow-service/src/config/loaders/quadstore-loader.ts (1)
9-16: LGTM: Well-organized graph naming constants.The named graph structure provides clear separation of concerns between platform defaults, input configurations, and merged results.
flow-service/src/config/resolution/service-config-accessor.ts (1)
7-21: LGTM: Clean constructor and basic property accessors.The constructor properly encapsulates the context, and the port/host getters use the utility function consistently.
flow-service/src/config/defaults.ts (2)
8-8: LGTM: Type imports updated consistently.The import changes from
NodeConfigtoMeshNodeConfigalign with the broader refactoring effort.
35-35: LGTM: Template paths corrected to absolute paths.The template paths are now properly using absolute paths under
/_assets/_templates/, which should resolve any relative path issues.Also applies to: 65-65
flow-service/src/config/loaders/jsonld-loader.ts (2)
8-12: LGTM: Import updates align with type refactoring.The import changes from
NodeConfigInputtoMeshNodeConfigInputand addition ofNodeObjectimport are consistent with the broader type system refactoring.
84-86: LGTM: Function signatures updated consistently.The renaming from
loadNodeConfig/saveNodeConfigtoloadMeshNodeConfig/saveMeshNodeConfigwith corresponding type updates maintains consistency across the codebase.Also applies to: 178-180
flow-service/src/config/resolution/service-config-resolver.ts (2)
1-15: LGTM! Clean modularization of utilities.The refactoring properly separates concerns by:
- Moving type definitions to
config-types.ts- Centralizing log level validation in
flow-core/src/platform-constants.ts- Extracting utility functions to
service-config-utils.tsThis improves maintainability and follows the single responsibility principle.
126-126: Good use of re-export pattern for backward compatibility.Re-exporting
getConfigValueandmergeConfigContextmaintains the existing API while allowing the implementation to be modularized.flow-service/src/config/config-types.ts (1)
8-18: LGTM! Good standardization with NodeObject base type.Using
NodeObjectandContextDefinitionfrom flow-core provides better type consistency across the codebase.flow-core/src/utils/quadstore-utils.ts (2)
8-15: LGTM! Clean stream counting implementation.The function correctly handles all stream events and follows the promise pattern properly.
76-92: LGTM! Efficient implementation using multiPut.The function correctly converts JSON-LD to quads and efficiently stores them using
multiPut.flow-service/src/config/index.ts (3)
9-27: LGTM! Clean reorganization of exports.The export changes properly reflect the refactoring:
- Types moved to
config-types.ts- Renamed types (e.g.,
FlowServiceContext,MeshNodeConfig) for clarity- Error types properly re-exported
59-66: Good modularization of service configuration utilities.The separation of utilities into focused modules improves maintainability:
service-config-utils.tsfor utility functionsservice-config-validator.tsfor validation logicservice-config-accessor.tsfor typed configuration access
85-118: Well-designed helper functions for configuration access.The two helper functions provide clean abstractions:
createServiceConfig: Returns an accessor for side-by-side configuration accessgetCompleteServiceConfig: Returns a fully merged configuration objectBoth properly handle errors and follow consistent patterns.
| export { Quadstore } from 'npm:quadstore'; | ||
| export { DataFactory } from 'npm:rdf-data-factory'; | ||
| export { MemoryLevel } from 'npm:memory-level'; | ||
|
|
||
| import jsonldDefault from "npm:jsonld"; | ||
| export const jsonld = jsonldDefault; | ||
|
|
||
| export { Engine } from 'npm:quadstore-comunica'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider pinning npm package versions.
The npm imports don't specify versions, which could lead to unexpected updates and potential breaking changes in production.
-export { Quadstore } from 'npm:quadstore';
-export { DataFactory } from 'npm:rdf-data-factory';
-export { MemoryLevel } from 'npm:memory-level';
+export { Quadstore } from 'npm:quadstore@^13.0.0';
+export { DataFactory } from 'npm:rdf-data-factory@^1.1.0';
+export { MemoryLevel } from 'npm:memory-level@^1.0.0';🌐 Web query:
What are the latest stable versions of quadstore, rdf-data-factory, memory-level, and quadstore-comunica npm packages?
💡 Result:
As of August 2, 2025, the latest stable versions of the requested npm packages are:
- quadstore: 15.3.0, published in June 2025[4][6].
- rdf-data-factory: 2.0.2, published in January 2025[2].
- memory-level: 3.1.0, published in April 2025[3].
- quadstore-comunica: The official Comunica engine tailored for quadstore is called quadstore-comunica, but the current published version is not explicitly listed in recent results. However, documentation for quadstore indicates that the quadstore-comunica package is regularly updated in concert with quadstore. The most reliable method to ensure compatibility is to use the latest version available on npm at this time, or check the dependencies listed in the quadstore documentation[1][4][9].
For explicit version info of quadstore-comunica, check the npm registry or use the following command:
npm view quadstore-comunica version
All package versions above are directly supported by the cited sources.
Citations:
- 1: https://www.npmjs.com/package/quadstore?activeTab=versions
- 2: https://www.npmjs.com/package/rdf-data-factory
- 3: https://npmjs.com/package/memory-level
- 4: https://www.npmjs.com/package/quadstore
- 5: https://www.npmjs.com/package/rdf-stores
- 6: https://www.npmjs.com/package/quadstore/v/7.2.0?activeTab=versions
- 7: https://classic.yarnpkg.com/en/package/rdf-quad
- 8: https://www.browserstack.com/guide/update-specific-package-using-npm
- 9: https://www.npmjs.com/package/quadstore/v/9.1.0
- 10: https://rdfjs.dev/quadstore
🌐 Web query:
What is the latest stable version of quadstore-comunica?
💡 Result:
The latest stable version of quadstore-comunica is 6.2.0, published on June 8, 2025[10][6].
Citations:
- 1: https://github.com/jacoscaz/quadstore-comunica
- 2: https://www.npmjs.com/package/quadstore/v/9.1.0
- 3: https://kgdev.net/products/quadstore-comunica/
- 4: https://www.npmjs.com/package/quadstore
- 5: https://www.npmjs.com/package/quadstore/v/4.0.1
- 6: https://codesandbox.io/examples/package/quadstore-comunica
- 7: https://www.npmjs.com/package/quadstore/v/7.0.1-beta.1?activeTab=code
- 8: Frontend usage with Vite? quadstorejs/quadstore#141
- 9: https://comunica.dev/docs/query/getting_started/query_dev_version/
- 10: https://www.npmjs.com/package/quadstore-comunica
Pin external npm imports to fixed versions
Locking these imports prevents unexpected upgrades and breaking changes:
- File: flow-core/src/deps.ts
Lines: 16–23
Suggested diff:
-export { Quadstore } from 'npm:quadstore';
-export { DataFactory } from 'npm:rdf-data-factory';
-export { MemoryLevel } from 'npm:memory-level';
+export { Quadstore } from 'npm:quadstore@^15.3.0';
+export { DataFactory } from 'npm:rdf-data-factory@^2.0.2';
+export { MemoryLevel } from 'npm:memory-level@^3.1.0';
-import jsonldDefault from "npm:jsonld";
+import jsonldDefault from "npm:jsonld@^1.0.0";
export const jsonld = jsonldDefault;
-export { Engine } from 'npm:quadstore-comunica';
+export { Engine } from 'npm:quadstore-comunica@^6.2.0';Adjust the jsonld version if you want to pin it similarly.
🤖 Prompt for AI Agents
In flow-core/src/deps.ts lines 16 to 23, the external npm imports are not pinned
to fixed versions, which can lead to unexpected upgrades and breaking changes.
Modify the import statements to include specific version numbers for each
package, including the jsonld import, by appending the version after the package
name (e.g., 'npm:package@version'). This will ensure consistent dependency
versions and prevent unintentional updates.
| export async function createNewGraphFromJsonLd( | ||
| inputJsonLd: NodeObject, | ||
| options: CreateGraphOptions = {} | ||
| ): Promise<number> { | ||
| const { | ||
| overwrite = false, | ||
| graphName = null, | ||
| bundle = defaultQuadstoreBundle, | ||
| } = options; | ||
|
|
||
| const { store, df } = bundle; | ||
| const graph = graphName ? df.namedNode(graphName) : df.defaultGraph(); | ||
| const quads = await jsonldToQuads(inputJsonLd, graph); | ||
| if (overwrite && graphName) clearGraph(df.namedNode(graphName)); | ||
|
|
||
| // Put quads into the store | ||
| await store.multiPut(quads); | ||
|
|
||
| return quads.length; | ||
| } |
There was a problem hiding this comment.
Critical: Missing await for clearGraph operation.
The clearGraph call on line 124 is not awaited, which could lead to race conditions where quads are added before the graph is fully cleared.
- if (overwrite && graphName) clearGraph(df.namedNode(graphName));
+ if (overwrite && graphName) {
+ await clearGraph(df.namedNode(graphName), bundle);
+ }📝 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.
| export async function createNewGraphFromJsonLd( | |
| inputJsonLd: NodeObject, | |
| options: CreateGraphOptions = {} | |
| ): Promise<number> { | |
| const { | |
| overwrite = false, | |
| graphName = null, | |
| bundle = defaultQuadstoreBundle, | |
| } = options; | |
| const { store, df } = bundle; | |
| const graph = graphName ? df.namedNode(graphName) : df.defaultGraph(); | |
| const quads = await jsonldToQuads(inputJsonLd, graph); | |
| if (overwrite && graphName) clearGraph(df.namedNode(graphName)); | |
| // Put quads into the store | |
| await store.multiPut(quads); | |
| return quads.length; | |
| } | |
| export async function createNewGraphFromJsonLd( | |
| inputJsonLd: NodeObject, | |
| options: CreateGraphOptions = {} | |
| ): Promise<number> { | |
| const { | |
| overwrite = false, | |
| graphName = null, | |
| bundle = defaultQuadstoreBundle, | |
| } = options; | |
| const { store, df } = bundle; | |
| const graph = graphName ? df.namedNode(graphName) : df.defaultGraph(); | |
| const quads = await jsonldToQuads(inputJsonLd, graph); | |
| if (overwrite && graphName) { | |
| await clearGraph(df.namedNode(graphName), bundle); | |
| } | |
| // Put quads into the store | |
| await store.multiPut(quads); | |
| return quads.length; | |
| } |
🤖 Prompt for AI Agents
In flow-core/src/utils/quadstore-utils.ts around lines 111 to 130, the
clearGraph call on line 124 is missing an await, causing potential race
conditions. Fix this by adding await before clearGraph to ensure the graph is
fully cleared before adding new quads.
flow-core/src/utils/rdfjs-utils.ts
Outdated
| export async function jsonldToQuads( | ||
| inputJsonLd: NodeObject, | ||
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | ||
| ): Promise<RDF.Quad[]> { | ||
| // Expand the JSON-LD object to full form | ||
| const expanded = await jsonld.expand(inputJsonLd); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON-LD expansion.
The jsonld.expand() call can throw errors for invalid JSON-LD input, but these are not handled. Consider wrapping in try-catch for better error reporting.
export async function jsonldToQuads(
inputJsonLd: NodeObject,
graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(),
): Promise<RDF.Quad[]> {
- // Expand the JSON-LD object to full form
- const expanded = await jsonld.expand(inputJsonLd);
+ // Expand the JSON-LD object to full form
+ let expanded;
+ try {
+ expanded = await jsonld.expand(inputJsonLd);
+ } catch (error) {
+ throw new Error(`Failed to expand JSON-LD: ${error.message}`);
+ }📝 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.
| export async function jsonldToQuads( | |
| inputJsonLd: NodeObject, | |
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | |
| ): Promise<RDF.Quad[]> { | |
| // Expand the JSON-LD object to full form | |
| const expanded = await jsonld.expand(inputJsonLd); | |
| export async function jsonldToQuads( | |
| inputJsonLd: NodeObject, | |
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | |
| ): Promise<RDF.Quad[]> { | |
| // Expand the JSON-LD object to full form | |
| let expanded; | |
| try { | |
| expanded = await jsonld.expand(inputJsonLd); | |
| } catch (error) { | |
| throw new Error(`Failed to expand JSON-LD: ${error.message}`); | |
| } |
🤖 Prompt for AI Agents
In flow-core/src/utils/rdfjs-utils.ts around lines 9 to 14, the call to
jsonld.expand() can throw errors if the input JSON-LD is invalid. Wrap the
jsonld.expand() call in a try-catch block to catch any exceptions, then log or
rethrow a more descriptive error to improve error reporting and debugging.
flow-core/src/utils/rdfjs-utils.ts
Outdated
| const quads: RDF.Quad[] = []; | ||
|
|
||
| for (const node of expanded) { | ||
| const subject = df.namedNode(node['@id'] as string); |
There was a problem hiding this comment.
Add validation for required @id field.
The function assumes all nodes have an @id field and casts it to string without validation. This could cause runtime errors if the JSON-LD is malformed.
- const subject = df.namedNode(node['@id'] as string);
+ if (!node['@id'] || typeof node['@id'] !== 'string') {
+ throw new Error(`Node missing required @id field: ${JSON.stringify(node)}`);
+ }
+ const subject = df.namedNode(node['@id']);📝 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.
| const subject = df.namedNode(node['@id'] as string); | |
| if (!node['@id'] || typeof node['@id'] !== 'string') { | |
| throw new Error(`Node missing required @id field: ${JSON.stringify(node)}`); | |
| } | |
| const subject = df.namedNode(node['@id']); |
🤖 Prompt for AI Agents
In flow-core/src/utils/rdfjs-utils.ts at line 20, the code assumes the node has
an '@id' field and casts it to string without validation, risking runtime errors
if '@id' is missing. Add a check to verify that node['@id'] exists and is a
string before using it to create the namedNode. If the validation fails, handle
the error appropriately, such as throwing an error or returning early, to
prevent runtime exceptions.
| if (cliOptions.sentryEnabled !== undefined) { | ||
| if (!config['fsvc:hasLoggingConfig']) { | ||
| config['fsvc:hasLoggingConfig'] = { | ||
| '@type': 'fsvc:LoggingConfig', | ||
| }; | ||
| } | ||
| const loggingConfig = config['fsvc:hasLoggingConfig'] as Record< | ||
| string, | ||
| unknown | ||
| >; | ||
| loggingConfig['fsvc:hasSentryChannel'] = { | ||
| '@type': 'fsvc:LogChannelConfig', | ||
| 'fsvc:logChannelEnabled': cliOptions.sentryEnabled, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential type safety issue with logging configuration mutation.
The type assertion to Record<string, unknown> on Line 81-84 bypasses TypeScript's type checking, which could lead to runtime errors if the structure is unexpected.
Use proper typing instead of type assertion:
if (cliOptions.sentryEnabled !== undefined) {
if (!config['fsvc:hasLoggingConfig']) {
config['fsvc:hasLoggingConfig'] = {
'@type': 'fsvc:LoggingConfig',
};
}
- const loggingConfig = config['fsvc:hasLoggingConfig'] as Record<
- string,
- unknown
- >;
- loggingConfig['fsvc:hasSentryChannel'] = {
+ config['fsvc:hasLoggingConfig']['fsvc:hasSentryChannel'] = {
'@type': 'fsvc:LogChannelConfig',
'fsvc:logChannelEnabled': cliOptions.sentryEnabled,
};
}📝 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 (cliOptions.sentryEnabled !== undefined) { | |
| if (!config['fsvc:hasLoggingConfig']) { | |
| config['fsvc:hasLoggingConfig'] = { | |
| '@type': 'fsvc:LoggingConfig', | |
| }; | |
| } | |
| const loggingConfig = config['fsvc:hasLoggingConfig'] as Record< | |
| string, | |
| unknown | |
| >; | |
| loggingConfig['fsvc:hasSentryChannel'] = { | |
| '@type': 'fsvc:LogChannelConfig', | |
| 'fsvc:logChannelEnabled': cliOptions.sentryEnabled, | |
| }; | |
| } | |
| if (cliOptions.sentryEnabled !== undefined) { | |
| if (!config['fsvc:hasLoggingConfig']) { | |
| config['fsvc:hasLoggingConfig'] = { | |
| '@type': 'fsvc:LoggingConfig', | |
| }; | |
| } | |
| config['fsvc:hasLoggingConfig']['fsvc:hasSentryChannel'] = { | |
| '@type': 'fsvc:LogChannelConfig', | |
| 'fsvc:logChannelEnabled': cliOptions.sentryEnabled, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In flow-service/src/config/resolution/service-config-loader.ts between lines 75
and 89, avoid using a broad type assertion to Record<string, unknown> for the
loggingConfig object as it bypasses TypeScript's type safety. Instead, define
and use a specific interface or type that accurately represents the expected
structure of the logging configuration. Replace the type assertion with this
proper type to ensure compile-time checks and prevent potential runtime errors
when mutating the loggingConfig object.
| export function getConfigValue<T>( | ||
| context: ServiceConfigContext, | ||
| inputKey: keyof ServiceConfigInput, | ||
| defaultKey: keyof typeof PLATFORM_SERVICE_DEFAULTS, | ||
| ): T { | ||
| const inputValue = context.inputOptions[inputKey]; | ||
| const defaultValue = context.defaultOptions[defaultKey]; | ||
|
|
||
| return (inputValue !== undefined ? inputValue : defaultValue) as T; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for getConfigValue function.
The generic type parameter T is unconstrained, which could lead to type mismatches. Consider constraining it to the actual value types or using a mapped type.
export function getConfigValue<T>(
context: ServiceConfigContext,
inputKey: keyof ServiceConfigInput,
- defaultKey: keyof typeof PLATFORM_SERVICE_DEFAULTS,
+ defaultKey: keyof typeof PLATFORM_SERVICE_DEFAULTS,
): T {
const inputValue = context.inputOptions[inputKey];
const defaultValue = context.defaultOptions[defaultKey];
- return (inputValue !== undefined ? inputValue : defaultValue) as T;
+ const result = inputValue !== undefined ? inputValue : defaultValue;
+ if (result === undefined) {
+ throw new Error(`Configuration value not found for keys: ${String(inputKey)}, ${String(defaultKey)}`);
+ }
+ return result as T;
}📝 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.
| export function getConfigValue<T>( | |
| context: ServiceConfigContext, | |
| inputKey: keyof ServiceConfigInput, | |
| defaultKey: keyof typeof PLATFORM_SERVICE_DEFAULTS, | |
| ): T { | |
| const inputValue = context.inputOptions[inputKey]; | |
| const defaultValue = context.defaultOptions[defaultKey]; | |
| return (inputValue !== undefined ? inputValue : defaultValue) as T; | |
| } | |
| export function getConfigValue<T>( | |
| context: ServiceConfigContext, | |
| inputKey: keyof ServiceConfigInput, | |
| defaultKey: keyof typeof PLATFORM_SERVICE_DEFAULTS, | |
| ): T { | |
| const inputValue = context.inputOptions[inputKey]; | |
| const defaultValue = context.defaultOptions[defaultKey]; | |
| const result = inputValue !== undefined ? inputValue : defaultValue; | |
| if (result === undefined) { | |
| throw new Error(`Configuration value not found for keys: ${String(inputKey)}, ${String(defaultKey)}`); | |
| } | |
| return result as T; | |
| } |
🤖 Prompt for AI Agents
In flow-service/src/config/resolution/service-config-utils.ts around lines 13 to
22, the generic type parameter T in getConfigValue is unconstrained, risking
type mismatches. To fix this, constrain T to the union of possible value types
from ServiceConfigInput and PLATFORM_SERVICE_DEFAULTS or use a mapped type that
derives T from the inputKey and defaultKey types, ensuring the returned value
type matches the expected configuration value type.
| if (!sentryDsn || !sentryDsn.startsWith('https://')) { | ||
| throw new ConfigError( | ||
| 'Sentry is enabled but no valid DSN is configured. Please set FLOW_SENTRY_DSN environment variable or configure it in the service config file.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Sentry DSN validation requirements.
The validation only checks that the DSN starts with "https://", but Sentry DSNs have a specific format. Consider using a more comprehensive validation or documenting why the simple prefix check is sufficient.
🏁 Script executed:
#!/bin/bash
# Search for other Sentry DSN validation patterns in the codebase
rg -A 3 -B 3 "sentry.*dsn|DSN.*sentry" --type tsLength of output: 2031
Enhance Sentry DSN validation or document rationale
The current check only ensures the DSN starts with “https://”, but Sentry DSNs have a more specific structure (https://<publicKey>@<host>/<projectId>). Please either tighten the validation or explicitly document why a simple prefix check is sufficient.
• File: flow-service/src/config/resolution/service-config-validator.ts
Lines 33–37
Suggested diff snippet:
- if (!sentryDsn || !sentryDsn.startsWith('https://')) {
+ // Sentry DSNs are of the form: https://<publicKey>@<host>/<projectId>
+ const sentryDsnPattern = /^https:\/\/[0-9a-zA-Z]+@[^/]+\/\d+$/;
+ if (!sentryDsn || !sentryDsnPattern.test(sentryDsn)) {Or, if you prefer to keep the simple check, add a comment above explaining that any HTTPS URL is acceptable and why more granular validation isn’t required.
📝 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 (!sentryDsn || !sentryDsn.startsWith('https://')) { | |
| throw new ConfigError( | |
| 'Sentry is enabled but no valid DSN is configured. Please set FLOW_SENTRY_DSN environment variable or configure it in the service config file.', | |
| ); | |
| } | |
| // Sentry DSNs are of the form: https://<publicKey>@<host>/<projectId> | |
| const sentryDsnPattern = /^https:\/\/[0-9a-zA-Z]+@[^/]+\/\d+$/; | |
| if (!sentryDsn || !sentryDsnPattern.test(sentryDsn)) { | |
| throw new ConfigError( | |
| 'Sentry is enabled but no valid DSN is configured. Please set FLOW_SENTRY_DSN environment variable or configure it in the service config file.', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In flow-service/src/config/resolution/service-config-validator.ts around lines
33 to 37, improve the Sentry DSN validation by either implementing a stricter
check that verifies the DSN matches the expected pattern
(https://<publicKey>@<host>/<projectId>) or, if keeping the current simple
check, add a clear comment above the validation explaining that any HTTPS URL is
acceptable and why more detailed validation is not necessary.
| // We'll need to extract them for testing | ||
|
|
||
| import { dirname, ensureDir } from '../../../flow-core/src/deps.ts'; | ||
| import { dirname, ensureDir } from '../../../../flow-core/src/deps.ts'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the updated import path for flow-core dependencies.
The import path was updated from ../../../flow-core/src/deps.ts to ../../../../flow-core/src/deps.ts, indicating the test file is now one directory level deeper relative to flow-core.
🏁 Script executed:
#!/bin/bash
# Verify the flow-core deps import path
test_file="flow-service/tests/integration/logging/formatting-direct.test.ts"
deps_file="flow-core/src/deps.ts"
echo "Test file location: $test_file"
echo "Deps file location: $deps_file"
# Check if the deps file exists and exports dirname, ensureDir
if [ -f "$deps_file" ]; then
echo "✅ Deps file exists at: $deps_file"
echo "Checking exports..."
rg "export.*dirname" "$deps_file" || echo "dirname export not found"
rg "export.*ensureDir" "$deps_file" || echo "ensureDir export not found"
else
echo "❌ Deps file not found at: $deps_file"
fiLength of output: 792
Missing dirname export in flow-core/src/deps.ts
The test now imports both dirname and ensureDir from flow-core/src/deps.ts, but only ensureDir is actually re-exported. You’ll need to add an export for dirname (or change the test to import it directly) to avoid a broken import.
• In flow-core/src/deps.ts: add
export { dirname } from "https://deno.land/std@0.224.0/path/mod.ts";so that the test’s import
import { dirname, ensureDir } from "../../../../flow-core/src/deps.ts";resolves correctly.
• Alternatively, update formatting-direct.test.ts to import dirname directly from the std path module instead of from deps.ts.
🤖 Prompt for AI Agents
In flow-core/src/deps.ts around line 1, add an export statement for dirname by
exporting it from "https://deno.land/std@0.224.0/path/mod.ts" to ensure it is
re-exported properly. This will allow the import in
flow-service/tests/integration/logging/formatting-direct.test.ts at line 10 to
work correctly without breaking. Alternatively, modify the test file to import
dirname directly from the std path module instead of from deps.ts.
| // Wait a moment for Sentry to initialize | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded delays with more reliable mechanisms.
The hardcoded timeouts (1000ms and 3000ms) make the test brittle and slow. Consider using Sentry's flush mechanism or event promises for more reliable synchronization.
- // Wait a moment for Sentry to initialize
- await new Promise((resolve) => setTimeout(resolve, 1000));
+ // Wait for Sentry to initialize
+ if (typeof Sentry !== 'undefined' && Sentry.flush) {
+ await Sentry.flush(2000);
+ }📝 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.
| // Wait a moment for Sentry to initialize | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| // Wait for Sentry to initialize | |
| if (typeof Sentry !== 'undefined' && Sentry.flush) { | |
| await Sentry.flush(2000); | |
| } |
🤖 Prompt for AI Agents
In flow-service/tests/integration/logging/sentry-init.test.ts at lines 44 to 45,
replace the hardcoded 1000ms delay used to wait for Sentry initialization with a
more reliable synchronization method such as Sentry's flush function or event
promises. This will make the test less brittle and faster by waiting explicitly
for Sentry to complete initialization instead of relying on arbitrary timeouts.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
flow-core/src/utils/rdfjs-utils.ts (1)
14-14: Add error handling for JSON-LD to RDF conversion.The
jsonld.toRDF()call can throw errors for invalid JSON-LD input, but these are not handled. The type casting toRDF.Quad[]also assumes the operation succeeds.Consider wrapping in try-catch for better error reporting:
- const quads = await jsonld.toRDF(inputJsonLd) as RDF.Quad[]; + let quads; + try { + quads = await jsonld.toRDF(inputJsonLd) as RDF.Quad[]; + } catch (error) { + throw new Error(`Failed to convert JSON-LD to RDF: ${error.message}`); + }flow-service/src/config/loaders/quadstore-loader.ts (1)
9-12: Add error handling for JSON-LD conversion operationsThe loader functions call
createNewGraphFromJsonLdwithout error handling. Malformed JSON-LD or store errors could cause uncaught exceptions.Wrap each call in try-catch blocks to provide context on failure:
export async function loadPlatformDefaults(): Promise<void> { - await createNewGraphFromJsonLd(PLATFORM_SERVICE_DEFAULTS, { graphName: CONFIG_GRAPH_NAMES.platformServiceDefaults }); - await createNewGraphFromJsonLd(PLATFORM_NODE_DEFAULTS, { graphName: CONFIG_GRAPH_NAMES.platformImplicitMeshRootNodeConfig }); + try { + await createNewGraphFromJsonLd(PLATFORM_SERVICE_DEFAULTS, { graphName: CONFIG_GRAPH_NAMES.platformServiceDefaults }); + await createNewGraphFromJsonLd(PLATFORM_NODE_DEFAULTS, { graphName: CONFIG_GRAPH_NAMES.platformImplicitMeshRootNodeConfig }); + } catch (error) { + console.error('Failed to load platform defaults:', error); + throw new Error(`Platform defaults loading failed: ${error.message}`); + } }Apply similar error handling to
loadInputServiceConfigandloadInputMeshRootNodeConfig.Also applies to: 17-19, 24-26, 28-54
flow-service/src/config/config-types.ts (1)
115-115: Fix property name inconsistency between interfaces.
ServiceConfiguses'fsvc:rootMeshRootNodeConfigTemplate'(line 82) whileServiceConfigInputstill uses'fsvc:nodeDefaults'(line 115). This inconsistency could cause runtime errors.Apply this diff to fix the inconsistency:
- 'fsvc:nodeDefaults'?: Partial<MeshRootNodeConfig>; + 'fsvc:rootMeshRootNodeConfigTemplate'?: Partial<MeshRootNodeConfig>;
🧹 Nitpick comments (3)
flow-service/tests/integration/quadstore-accessor.test.ts (1)
37-40: Consider adding error handling for query execution.The query execution assumes success but could benefit from error handling for more robust testing.
const result = []; const bindingsStream = await engine!.queryBindings(query, { sources: [store] }); - for await (const binding of bindingsStream as any) { - result.push(binding.get("o").value); - } + try { + for await (const binding of bindingsStream as any) { + result.push(binding.get("o").value); + } + } catch (error) { + throw new Error(`SPARQL query execution failed: ${error.message}`); + }flow-service/src/routes/meshes.ts (1)
131-136: Improve string formatting for readabilityConsider using template literals for cleaner formatting:
- message: `Mesh '${name}' is already registered at path: ${meshRegistry[name] - }`, + message: `Mesh '${name}' is already registered at path: ${meshRegistry[name]}`,flow-service/src/utils/startup-logger.ts (1)
42-51: Address TODO comments for configuration propertiesThe hardcoded values and commented code indicate missing accessor methods for logging levels and service enablement flags.
Would you like me to help implement the missing accessor methods in the
ServiceConfigAccessorclass or create an issue to track this technical debt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
deno.lockis excluded by!**/*.lockflow-service/deno.lockis excluded by!**/*.lockflow-service/logs/test-pretty-direct.logis excluded by!**/*.logpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
.ai-guidance.md(1 hunks).gitignore(1 hunks)deno.json(1 hunks)eslint.config.mjs(1 hunks)flow-core/src/deps.ts(1 hunks)flow-core/src/utils/quadstore-utils.ts(1 hunks)flow-core/src/utils/quadstore/quadstore-factory.ts(1 hunks)flow-core/src/utils/rdfjs-utils.ts(1 hunks)flow-core/tests/unit/quadstore-utils.test.ts(1 hunks)flow-service/deno.json(1 hunks)flow-service/flow-service-config.jsonld(1 hunks)flow-service/main.ts(5 hunks)flow-service/src/config/config-types.ts(6 hunks)flow-service/src/config/defaults.ts(2 hunks)flow-service/src/config/index.ts(2 hunks)flow-service/src/config/loaders/env-loader.ts(2 hunks)flow-service/src/config/loaders/jsonld-loader.ts(7 hunks)flow-service/src/config/loaders/quadstore-loader.ts(1 hunks)flow-service/src/config/node-config.default.jsonld(1 hunks)flow-service/src/config/resolution/service-config-accessor.ts(1 hunks)flow-service/src/config/resolution/service-config-loader.ts(1 hunks)flow-service/src/config/resolution/service-config-resolver.ts(1 hunks)flow-service/src/config/resolution/service-config-validator.ts(1 hunks)flow-service/src/quadstore-default-bundle.ts(1 hunks)flow-service/src/routes/meshes.ts(2 hunks)flow-service/src/routes/weave.ts(1 hunks)flow-service/src/services/metadata-composer.ts(1 hunks)flow-service/src/utils/logger.ts(1 hunks)flow-service/src/utils/mesh-utils.ts(2 hunks)flow-service/src/utils/startup-logger.ts(3 hunks)flow-service/tests/integration/logging/formatting-direct.test.ts(3 hunks)flow-service/tests/integration/logging/log-retention-config.test.ts(1 hunks)flow-service/tests/integration/logging/sentry-init.test.ts(1 hunks)flow-service/tests/integration/logging/sentry-logging.test.ts(1 hunks)flow-service/tests/integration/quadstore-accessor.test.ts(1 hunks)flow-service/tests/integration/quadstore-utils.test.ts(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- .gitignore
- .ai-guidance.md
- package.json
- flow-service/src/quadstore-default-bundle.ts
- deno.json
- flow-service/src/utils/logger.ts
- flow-service/tests/integration/logging/sentry-init.test.ts
- flow-core/src/utils/quadstore/quadstore-factory.ts
- flow-service/flow-service-config.jsonld
- flow-service/src/services/metadata-composer.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- flow-service/tests/integration/logging/sentry-logging.test.ts
- flow-service/deno.json
- flow-service/src/config/node-config.default.jsonld
- flow-service/tests/integration/logging/log-retention-config.test.ts
- flow-service/src/config/resolution/service-config-validator.ts
- flow-service/tests/integration/logging/formatting-direct.test.ts
- flow-core/tests/unit/quadstore-utils.test.ts
- flow-service/src/config/resolution/service-config-loader.ts
- flow-core/src/deps.ts
- flow-service/src/config/loaders/env-loader.ts
- flow-service/src/config/loaders/jsonld-loader.ts
- flow-core/src/utils/quadstore-utils.ts
- flow-service/src/config/defaults.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
flow-core/src/utils/rdfjs-utils.ts (2)
flow-core/src/deps.ts (3)
DataFactory(17-17)NodeObject(31-31)jsonld(21-21)flow-service/src/config/types.ts (1)
JSONLDContext(9-15)
flow-service/tests/integration/quadstore-utils.test.ts (2)
flow-core/src/deps.ts (4)
MemoryLevel(18-18)DataFactory(17-17)Quadstore(16-16)assertEquals(3-3)flow-core/src/utils/quadstore-utils.ts (3)
clearGraph(35-50)copyGraph(54-70)countQuadsInStream(8-15)
🔇 Additional comments (26)
flow-service/src/routes/weave.ts (2)
19-20: Function signature correctly simplified for singleton pattern.The removal of the
configparameter aligns with the singleton configuration accessor pattern described in the PR objectives. However, ensure that any configuration access within this route (if needed in the future) uses the singleton accessor.
92-95: Minor formatting improvement.The consolidation of the multiline template literal into a single line improves readability while maintaining the same logged content.
eslint.config.mjs (1)
1-19: ESLint configuration looks good.The configuration properly targets TypeScript files and enforces consistent code style with double quotes and quoted object properties. The flat config format is correct and the parser options are appropriate.
flow-core/src/utils/rdfjs-utils.ts (1)
22-24: Graph remapping logic is correct.The logic for remapping quads to a specific named graph when requested is implemented correctly using the DataFactory.
flow-service/src/utils/mesh-utils.ts (2)
3-3: Import change aligns with singleton pattern.The import of the singleton service config accessor is consistent with the refactoring described in the PR objectives.
29-31: Minor formatting improvement.The consolidation of the warning message into a single line improves readability while maintaining the same content.
flow-service/tests/integration/quadstore-accessor.test.ts (1)
5-44: Well-structured integration test.The test properly validates the Quadstore SPARQL query functionality with good practices:
- Proper setup and teardown with graph clearing
- Realistic test data insertion via JSON-LD
- Comprehensive SPARQL query testing with named graphs
- Appropriate assertions
flow-service/main.ts (2)
14-22: Configuration initialization pattern looks goodThe refactoring to use a singleton configuration accessor is well implemented. The initialization is properly wrapped in a try-catch block with appropriate error handling and process termination on failure.
85-86: Clean refactoring of route creationRemoving the config parameter from route creation functions reduces coupling and simplifies the API. The routes can now access configuration through the singleton accessor internally when needed.
flow-service/src/routes/meshes.ts (1)
18-18: Function signature simplified appropriatelyThe removal of the config parameter aligns with the singleton pattern adoption throughout the codebase, reducing coupling and simplifying the API.
flow-service/tests/integration/quadstore-utils.test.ts (1)
1-56: Well-structured integration test for quadstore utilitiesThe test comprehensively covers the clearGraph and copyGraph utilities with proper setup, execution, and verification steps. Good use of a helper function to avoid code duplication.
flow-service/src/config/loaders/quadstore-loader.ts (1)
31-54: Well-implemented configuration merge logicThe merge function correctly:
- Clears the target graph before merging
- Prioritizes input config over platform defaults
- Checks for existence before adding platform defaults to avoid duplicates
- Provides helpful debug logging
flow-service/src/utils/startup-logger.ts (1)
16-16: Correct async function conversionThe functions are properly converted to async to accommodate the singleton configuration accessor's async methods.
Also applies to: 62-65
flow-service/src/config/resolution/service-config-accessor.ts (4)
7-14: LGTM! Clean singleton pattern implementation.The anonymous class with immediate instantiation provides a clean singleton pattern. The constructor properly initializes the bundle and DataFactory dependencies.
17-34: Well-structured helper method with proper error handling.The
querySingleValuemethod properly checks for engine initialization, handles SPARQL execution errors with logging, and gracefully returns undefined when no results are found.
36-47: Good implementation with proper type conversion.The
getPortmethod correctly uses the helper method and converts the string result to a number. The SPARQL query is well-structured with proper graph scoping.
49-59: LGTM! Consistent implementation pattern.The
getHostmethod follows the same pattern asgetPortand properly delegates to the helper method.flow-service/src/config/resolution/service-config-resolver.ts (3)
10-23: LGTM! Well-organized imports for the refactored architecture.The import statements properly reference the new modular structure with Quadstore loaders and external validation functions. The organization is clean and logical.
24-71: Excellent architectural improvement with Quadstore-based configuration.The refactored function demonstrates a much cleaner separation of concerns:
- Modular loading: Platform defaults, environment, file, and CLI configs are loaded separately
- Sequential merging: Input configs are properly merged with precedence (env < file < CLI)
- Quadstore integration: Configuration data is loaded into graphs for SPARQL querying
- Simplified logic: Removed complex inline merging in favor of dedicated loader functions
The error handling remains robust with proper ConfigError wrapping and logging.
81-126: LGTM! Improved CLI options conversion with external validation.The function correctly converts CLI options to the internal configuration format. The use of external
validateLogLevelfrom flow-core improves consistency and centralized validation logic. The nested logging configuration structure is properly constructed.flow-service/src/config/config-types.ts (3)
8-17: Excellent adoption of standard JSON-LD types.The migration from custom
JSONLDBaseto the standardNodeObjectfrom@types/jsonldimproves type safety and aligns with JSON-LD best practices. TheFlowServiceContextproperly extendsContextDefinition.
20-72: LGTM! Consistent interface updates with NodeObject extension.All configuration interfaces properly extend
NodeObject, maintaining type safety while aligning with JSON-LD standards. The renaming fromNodeConfigtoMeshRootNodeConfigimproves clarity about the interface's purpose.
75-193: LGTM! Consistent type definitions with proper NodeObject extension.All remaining interfaces properly extend
NodeObjectand maintain semantic consistency. The error classes, utility types, and environment configuration remain appropriately defined.flow-service/src/config/index.ts (3)
8-19: LGTM! Well-structured graph naming for Quadstore configuration.The
CONFIG_GRAPH_NAMESconstant provides centralized naming for configuration graphs with clear, semantic identifiers. The localhost URIs are appropriate for internal graph references.
22-82: Excellent export organization reflecting the architectural refactoring.The exports properly reflect the new modular structure:
- Updated type names: Consistent with the MeshRootNodeConfig renaming
- New modules: Validation and singleton accessor properly exported
- Removed helpers: Clean removal of functions now handled by the singleton
- Consistent naming: JSON-LD loader exports use consistent naming
84-99: LGTM! Clean separation of concerns in configuration creation.The updated
createServiceConfigfunction properly orchestrates the new configuration workflow:
- Clear separation: Resolution and validation are handled by dedicated functions
- Proper sequencing: Configuration is resolved before validation
- Consistent error handling: Maintains the same error handling pattern
- Return type: Correctly returns void since configuration access is now via singleton
| export async function jsonldToQuads( | ||
| inputJsonLd: NodeObject, | ||
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | ||
| ): Promise<RDF.Quad[]> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for JSON-LD parameter.
The function accepts any NodeObject but doesn't validate that it's valid JSON-LD. Consider adding basic validation to ensure the input is well-formed.
export async function jsonldToQuads(
inputJsonLd: NodeObject,
graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(),
): Promise<RDF.Quad[]> {
+ if (!inputJsonLd || typeof inputJsonLd !== 'object') {
+ throw new Error('Input must be a valid JSON-LD object');
+ }📝 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.
| export async function jsonldToQuads( | |
| inputJsonLd: NodeObject, | |
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | |
| ): Promise<RDF.Quad[]> { | |
| export async function jsonldToQuads( | |
| inputJsonLd: NodeObject, | |
| graph: RDF.NamedNode | RDF.DefaultGraph = df.defaultGraph(), | |
| ): Promise<RDF.Quad[]> { | |
| if (!inputJsonLd || typeof inputJsonLd !== 'object') { | |
| throw new Error('Input must be a valid JSON-LD object'); | |
| } | |
| // …rest of the implementation… |
🤖 Prompt for AI Agents
In flow-core/src/utils/rdfjs-utils.ts around lines 9 to 12, the jsonldToQuads
function accepts a NodeObject input without validating if it is well-formed
JSON-LD. Add input validation at the start of the function to check that
inputJsonLd conforms to expected JSON-LD structure, such as verifying required
properties or types. If validation fails, throw an appropriate error to prevent
processing invalid data.
| // Log service startup with configuration info | ||
| try { | ||
| logStartupConfiguration(config); | ||
| logStartupConfiguration(); |
There was a problem hiding this comment.
Add missing await keywords for async logging functions
The logStartupConfiguration() and logStartupUrls() functions were converted to async but are called without await. This could lead to:
- Uncaught promise rejections if these functions throw errors
- Race conditions where logging output appears out of order
- The try-catch blocks not properly catching errors from these async operations
Apply this fix:
- logStartupConfiguration();
+ await logStartupConfiguration();And on line 93:
- logStartupUrls();
+ await logStartupUrls();Also applies to: 93-93
🤖 Prompt for AI Agents
In flow-service/main.ts at lines 26 and 93, the async functions
logStartupConfiguration() and logStartupUrls() are called without await, which
can cause unhandled promise rejections and race conditions. Fix this by adding
the await keyword before both function calls to ensure proper error handling and
sequential execution within their respective try-catch blocks.
| async getMeshPaths(): Promise<string[]> { | ||
| const sparql = ` | ||
| PREFIX fsvc: <https://semantic-flow.github.io/ontology/flow-service/> | ||
| SELECT ?value WHERE { | ||
| GRAPH <${CONFIG_GRAPH_NAMES.mergedServiceConfig}> { | ||
| ?s fsvc:meshPaths ?value . | ||
| } | ||
| } | ||
| `; | ||
| if (!this.bundle.engine) { | ||
| throw new Error('SPARQL engine not initialized in Quadstore bundle'); | ||
| } | ||
| const values: string[] = []; | ||
| const bindingsStream = await this.bundle.engine.queryBindings(sparql, { sources: [this.bundle.store] }); | ||
| for await (const binding of bindingsStream as any) { | ||
| const value = binding.get('value'); | ||
| if (value) { | ||
| values.push(value.value); | ||
| } | ||
| } | ||
| return values; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication by refactoring to use existing helper.
The getMeshPaths method duplicates engine checking and query execution logic that already exists in querySingleValue. This violates the DRY principle.
Refactor to use a more generic helper method:
- async getMeshPaths(): Promise<string[]> {
- const sparql = `
- PREFIX fsvc: <https://semantic-flow.github.io/ontology/flow-service/>
- SELECT ?value WHERE {
- GRAPH <${CONFIG_GRAPH_NAMES.mergedServiceConfig}> {
- ?s fsvc:meshPaths ?value .
- }
- }
- `;
- if (!this.bundle.engine) {
- throw new Error('SPARQL engine not initialized in Quadstore bundle');
- }
- const values: string[] = [];
- const bindingsStream = await this.bundle.engine.queryBindings(sparql, { sources: [this.bundle.store] });
- for await (const binding of bindingsStream as any) {
- const value = binding.get('value');
- if (value) {
- values.push(value.value);
- }
- }
- return values;
- }
+ private async queryMultipleValues(sparql: string): Promise<string[]> {
+ if (!this.bundle.engine) {
+ throw new Error('SPARQL engine not initialized in Quadstore bundle');
+ }
+ try {
+ const values: string[] = [];
+ const bindingsStream = await this.bundle.engine.queryBindings(sparql, { sources: [this.bundle.store] });
+ for await (const binding of bindingsStream as any) {
+ const value = binding.get('value');
+ if (value) {
+ values.push(value.value);
+ }
+ }
+ return values;
+ } catch (error) {
+ await handleCaughtError(error, 'Failed to execute SPARQL query');
+ throw new Error('Failed to execute SPARQL query');
+ }
+ }
+
+ async getMeshPaths(): Promise<string[]> {
+ const sparql = `
+ PREFIX fsvc: <https://semantic-flow.github.io/ontology/flow-service/>
+ SELECT ?value WHERE {
+ GRAPH <${CONFIG_GRAPH_NAMES.mergedServiceConfig}> {
+ ?s fsvc:meshPaths ?value .
+ }
+ }
+ `;
+ return this.queryMultipleValues(sparql);
+ }📝 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.
| async getMeshPaths(): Promise<string[]> { | |
| const sparql = ` | |
| PREFIX fsvc: <https://semantic-flow.github.io/ontology/flow-service/> | |
| SELECT ?value WHERE { | |
| GRAPH <${CONFIG_GRAPH_NAMES.mergedServiceConfig}> { | |
| ?s fsvc:meshPaths ?value . | |
| } | |
| } | |
| `; | |
| if (!this.bundle.engine) { | |
| throw new Error('SPARQL engine not initialized in Quadstore bundle'); | |
| } | |
| const values: string[] = []; | |
| const bindingsStream = await this.bundle.engine.queryBindings(sparql, { sources: [this.bundle.store] }); | |
| for await (const binding of bindingsStream as any) { | |
| const value = binding.get('value'); | |
| if (value) { | |
| values.push(value.value); | |
| } | |
| } | |
| return values; | |
| } | |
| private async queryMultipleValues(sparql: string): Promise<string[]> { | |
| if (!this.bundle.engine) { | |
| throw new Error('SPARQL engine not initialized in Quadstore bundle'); | |
| } | |
| try { | |
| const values: string[] = []; | |
| const bindingsStream = await this.bundle.engine.queryBindings(sparql, { sources: [this.bundle.store] }); | |
| for await (const binding of bindingsStream as any) { | |
| const value = binding.get('value'); | |
| if (value) { | |
| values.push(value.value); | |
| } | |
| } | |
| return values; | |
| } catch (error) { | |
| await handleCaughtError(error, 'Failed to execute SPARQL query'); | |
| throw new Error('Failed to execute SPARQL query'); | |
| } | |
| } | |
| async getMeshPaths(): Promise<string[]> { | |
| const sparql = ` | |
| PREFIX fsvc: <https://semantic-flow.github.io/ontology/flow-service/> | |
| SELECT ?value WHERE { | |
| GRAPH <${CONFIG_GRAPH_NAMES.mergedServiceConfig}> { | |
| ?s fsvc:meshPaths ?value . | |
| } | |
| } | |
| `; | |
| return this.queryMultipleValues(sparql); | |
| } |
🤖 Prompt for AI Agents
In flow-service/src/config/resolution/service-config-accessor.ts around lines 61
to 82, the getMeshPaths method duplicates the SPARQL engine initialization check
and query execution logic already implemented in the querySingleValue helper.
Refactor getMeshPaths to call querySingleValue with the appropriate SPARQL query
and process its result to return the mesh paths, thereby eliminating duplicated
code and adhering to the DRY principle.
| meshRegistry: Record<string, string>, | ||
| ) => { | ||
| const meshPaths = config.meshPaths; | ||
| const meshPaths = config.getMeshPaths; |
There was a problem hiding this comment.
Critical bug: Missing function call and await.
Line 8 assigns config.getMeshPaths (the method reference) instead of calling config.getMeshPaths(). Since this is an asynchronous method, the function signature also needs to be updated to be async and await the call.
Apply this fix:
-export const initializeMeshRegistry = (
+export const initializeMeshRegistry = async (
meshRegistry: Record<string, string>,
) => {
- const meshPaths = config.getMeshPaths;
+ const meshPaths = await config.getMeshPaths();🤖 Prompt for AI Agents
In flow-service/src/utils/mesh-utils.ts at line 8, the code assigns the method
reference config.getMeshPaths instead of calling it. Update the function
containing this line to be async, call config.getMeshPaths() with parentheses,
and await its result to properly handle the asynchronous call.
1 participant
@CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
.gitignoreto exclude logs directory.package.jsonwith ESLint dependencies.Documentation