feat: multi-pattern glob support for observers and queries; v12.1.0#49
feat: multi-pattern glob support for observers and queries; v12.1.0#49flyingrobots wants to merge 3 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces multi-pattern glob support for graph observers, queries, and watchers. A new centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Release Preflight
If you tag this commit as |
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/domain/warp/subscribe.methods.js (1)
111-113:⚠️ Potential issue | 🟡 MinorUpdate
@throwsdocumentation to reflect array support.The
@throwsannotation on line 111 says "If pattern is not a string" but the validation now accepts arrays of strings. The error message on line 136 is correct, but the JSDoc is stale.📝 Suggested fix
- * `@throws` {Error} If pattern is not a string + * `@throws` {Error} If pattern is not a string or array of strings🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/warp/subscribe.methods.js` around lines 111 - 113, Update the JSDoc `@throws` line that currently reads "If pattern is not a string" to reflect that the function accepts either a string or an array of strings (e.g., "If pattern is not a string or an array of strings"); locate the JSDoc above the subscribe-related function in src/domain/warp/subscribe.methods.js (the same block that documents the throws for onChange and poll) and change the wording so it matches the runtime validation/error message emitted around the validation at line ~136.
🧹 Nitpick comments (3)
src/domain/warp/query.methods.js (1)
334-339: JSDoc type annotations are inconsistent with the actual implementation.The JSDoc for
translationCostparameters still shows{string}forconfigA.matchandconfigB.match, but the implementation (viacomputeTranslationCost) acceptsstring | string[]. Consider updating for consistency:* `@param` {Object} configA - Observer configuration for A - * `@param` {string} configA.match - Glob pattern for visible nodes + * `@param` {string|string[]} configA.match - Glob pattern(s) for visible nodes * `@param` {string[]} [configA.expose] - Property keys to include * `@param` {string[]} [configA.redact] - Property keys to exclude * `@param` {Object} configB - Observer configuration for B - * `@param` {string} configB.match - Glob pattern for visible nodes + * `@param` {string|string[]} configB.match - Glob pattern(s) for visible nodes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/warp/query.methods.js` around lines 334 - 339, The JSDoc for translationCost is inaccurate: update the types for configA.match and configB.match (and any related param docs like in the config objects) from {string} to {string|string[]} to match the implementation in computeTranslationCost; ensure any occurrences in the JSDoc for translationCost and its configA/configB parameter descriptions reflect the union type and mirror computeTranslationCost’s accepted input shape (including optional expose/redact arrays).src/domain/utils/matchGlob.js (1)
1-2: Consider adding a cache size limit to prevent unbounded memory growth.The
globRegexCachegrows indefinitely. If patterns are user-controlled or dynamic, this could lead to memory leaks. For a library with long-running processes, consider adding an LRU eviction strategy or a maximum cache size.That said, for typical usage with a fixed set of patterns (e.g., observer configs), the current implementation is likely fine.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/utils/matchGlob.js` around lines 1 - 2, The module-level Map globRegexCache currently grows without bound; add a bounded cache policy (e.g., LRU or max-size eviction) around accesses that insert into globRegexCache so that when inserting a new compiled RegExp you evict the least-recently-used or oldest entry once a configured MAX_CACHE_SIZE is exceeded; update the code paths that read/insert into globRegexCache so they mark entries as recently used (or move them to the front) and make MAX_CACHE_SIZE configurable and documented.src/domain/services/ObserverView.js (1)
167-182: Consider adding validation forconfig.matchin constructor.The
ObserverViewconstructor acceptsconfig.matchwithout validating it's a string or string array. While the caller (graph.observer()) likely validates this, adding defensive validation here would prevent subtle bugs if the class is instantiated directly.This is a minor suggestion since the internal matchGlob utility handles non-string patterns gracefully (returns
false), but explicit validation would provide clearer error messages at construction time rather than silent mismatches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/services/ObserverView.js` around lines 167 - 182, Add defensive validation in the ObserverView constructor for config.match: check that config.match is either a string or an array of strings (use typeof and Array.isArray with element type checks) before assigning to this._matchPattern, and throw a clear TypeError if it is missing or of the wrong type; update the constructor (where this._matchPattern is assigned) to perform this validation so direct instantiation of ObserverView fails fast with a helpful message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 17: The version header for 12.0.0 in CHANGELOG.md uses "### [12.0.0] —
2026-02-25" which is an h3 and inconsistent with the rest of the changelog;
change that header to "## [12.0.0] — 2026-02-25" so it matches the h2 format
used by other release sections and conforms to Keep a Changelog.
---
Outside diff comments:
In `@src/domain/warp/subscribe.methods.js`:
- Around line 111-113: Update the JSDoc `@throws` line that currently reads "If
pattern is not a string" to reflect that the function accepts either a string or
an array of strings (e.g., "If pattern is not a string or an array of strings");
locate the JSDoc above the subscribe-related function in
src/domain/warp/subscribe.methods.js (the same block that documents the throws
for onChange and poll) and change the wording so it matches the runtime
validation/error message emitted around the validation at line ~136.
---
Nitpick comments:
In `@src/domain/services/ObserverView.js`:
- Around line 167-182: Add defensive validation in the ObserverView constructor
for config.match: check that config.match is either a string or an array of
strings (use typeof and Array.isArray with element type checks) before assigning
to this._matchPattern, and throw a clear TypeError if it is missing or of the
wrong type; update the constructor (where this._matchPattern is assigned) to
perform this validation so direct instantiation of ObserverView fails fast with
a helpful message.
In `@src/domain/utils/matchGlob.js`:
- Around line 1-2: The module-level Map globRegexCache currently grows without
bound; add a bounded cache policy (e.g., LRU or max-size eviction) around
accesses that insert into globRegexCache so that when inserting a new compiled
RegExp you evict the least-recently-used or oldest entry once a configured
MAX_CACHE_SIZE is exceeded; update the code paths that read/insert into
globRegexCache so they mark entries as recently used (or move them to the front)
and make MAX_CACHE_SIZE configurable and documented.
In `@src/domain/warp/query.methods.js`:
- Around line 334-339: The JSDoc for translationCost is inaccurate: update the
types for configA.match and configB.match (and any related param docs like in
the config objects) from {string} to {string|string[]} to match the
implementation in computeTranslationCost; ensure any occurrences in the JSDoc
for translationCost and its configA/configB parameter descriptions reflect the
union type and mirror computeTranslationCost’s accepted input shape (including
optional expose/redact arrays).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.mdindex.d.tspackage.jsonsrc/domain/services/ObserverView.jssrc/domain/services/QueryBuilder.jssrc/domain/services/TranslationCost.jssrc/domain/utils/matchGlob.jssrc/domain/warp/query.methods.jssrc/domain/warp/subscribe.methods.jstest/unit/domain/WarpGraph.queryBuilder.test.jstest/unit/domain/services/ObserverView.test.jstest/unit/domain/services/TranslationCost.test.js
| - **Multi-pattern glob support** — `graph.observer()`, `query().match()`, and `translationCost()` now accept an array of glob patterns (e.g. `['campaign:*', 'milestone:*']`). Nodes matching *any* pattern in the array are included (OR semantics). | ||
| - **Centralized `matchGlob` utility** (`src/domain/utils/matchGlob.js`) — unified glob matching logic with regex caching and support for array-based multi-pattern matching. | ||
|
|
||
| ### [12.0.0] — 2026-02-25 |
There was a problem hiding this comment.
Fix version header formatting for 12.0.0.
The 12.0.0 section uses ### (h3) instead of ## (h2), which breaks consistency with other version headers in this changelog and violates Keep a Changelog format.
📝 Suggested fix
-### [12.0.0] — 2026-02-25
+## [12.0.0] — 2026-02-25📝 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.
| ### [12.0.0] — 2026-02-25 | |
| ## [12.0.0] — 2026-02-25 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 17, The version header for 12.0.0 in CHANGELOG.md uses
"### [12.0.0] — 2026-02-25" which is an h3 and inconsistent with the rest of the
changelog; change that header to "## [12.0.0] — 2026-02-25" so it matches the h2
format used by other release sections and conforms to Keep a Changelog.
Centralizes glob matching logic into a new
matchGlobutility and adds support for array-based multi-prefix matching ingraph.observer(),query().match(),translationCost(), andwatch(). Includes version bump to v12.1.0 and updated tests.Summary by CodeRabbit
Release Notes
graph.observer(),query().match(),translationCost(), andwatch()now accept either a single string or an array of glob patterns. Nodes matching any pattern in the array are included with OR semantics.