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 merged 5 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
|
@coderabbitai review please 🥕 |
|
This PR introduces multi-pattern glob support across the codebase with a centralized ✅ Actions performedReview triggered.
|
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)
141-143:⚠️ Potential issue | 🟠 MajorHarden poll validation for non-finite numbers and avoid truthiness gating.
NaNandInfinityboth passtypeof poll === 'number', and sinceNaN < 1000andInfinity < 1000both evaluate tofalse, the validation at line 142 doesn't catch them. Then line 189'sif (poll)causesNaNto be silently skipped (falsy) andInfinityto pass through tosetInterval(truthy), creating unexpected behavior. UseNumber.isFinite()to reject non-finite numbers and gate onpoll !== undefinedinstead of truthiness.Suggested fix
if (poll !== undefined) { - if (typeof poll !== 'number' || poll < 1000) { + if (typeof poll !== 'number' || !Number.isFinite(poll) || poll < 1000) { throw new Error('poll must be a number >= 1000'); } } @@ - if (poll) { + if (poll !== undefined) { pollIntervalId = setInterval(() => {🤖 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 141 - 143, The poll validation is too weak: update the check around the poll variable in the subscribe flow (where poll is currently validated and later used in setInterval) to reject non-finite numbers and avoid truthiness gating; specifically, replace the typeof/less-than check with Number.isFinite(poll) and ensure the runtime branch uses poll !== undefined (not if (poll)) before calling setInterval, and update the thrown error message to state "poll must be a finite number >= 1000".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/domain/warp/subscribe.methods.js`:
- Around line 134-137: The current isValidPattern check accepts empty or sparse
arrays (e.g., [] or [ , ]) which creates no-op watchers; update validation
around pattern (used in isValidPattern and the subsequent throw) to reject empty
arrays and arrays with holes by requiring Array.isArray(pattern) &&
pattern.length > 0 && Object.keys(pattern).length === pattern.length &&
pattern.every(i => typeof i === 'string'), so only non-empty, dense arrays of
strings (or a string) are allowed.
---
Outside diff comments:
In `@src/domain/warp/subscribe.methods.js`:
- Around line 141-143: The poll validation is too weak: update the check around
the poll variable in the subscribe flow (where poll is currently validated and
later used in setInterval) to reject non-finite numbers and avoid truthiness
gating; specifically, replace the typeof/less-than check with
Number.isFinite(poll) and ensure the runtime branch uses poll !== undefined (not
if (poll)) before calling setInterval, and update the thrown error message to
state "poll must be a finite number >= 1000".
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
index.d.tssrc/domain/warp/subscribe.methods.js
🚧 Files skipped from review as they are similar to previous changes (1)
- index.d.ts
Release Preflight
If you tag this commit as |
- Reject empty arrays in match pattern validators (observer, watch, translationCost) — prevents no-op watchers - Reject NaN/Infinity in poll validation via Number.isFinite() - Replace truthiness gate `if (poll)` with `if (poll !== undefined)` - Add tests for NaN, Infinity, and empty array edge cases
|
@coderabbitai review please 🥕 |
|
This PR introduces multi-pattern glob support across the codebase with a centralized ✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
1 participant
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
New Features
watchnow accepts either a single pattern or an array of patterns, enabling monitoring of multiple patterns in one call.Bug Fixes / Validation
Documentation