refactor: Package dependencies for indy-charts#472
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: facioquo/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
✨ Finishing Touches✨ Simplify code
|
Deploying stock-charts with
|
| Latest commit: |
5ea6690
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1a9ecc3d.stock-charts.pages.dev |
| Branch Preview URL: | https://update-node-packages.stock-charts.pages.dev |
Deploying stock-charts-vitepress with
|
| Latest commit: |
5ea6690
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bb244e28.stock-charts-vitepress.pages.dev |
| Branch Preview URL: | https://update-node-packages.stock-charts-vitepress.pages.dev |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | -435 |
| Duplication | 17 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
8-10: ⚡ Quick winConsider re-adding the pnpm engine constraint for pnpm v11.
The
enginesfield previously enforcedpnpm>=10but now only specifies Node>=24. SincepackageManageron line 7 pins topnpm@11.1.2and the PR upgrades the entire monorepo to v11 (including breaking syntax changes inpnpm-workspace.yaml), explicitly requiringpnpm>=11would prevent accidental usage of older pnpm versions that won't understand the new workspace configuration.💡 Suggested addition
"engines": { - "node": ">=24" + "node": ">=24", + "pnpm": ">=11" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 8 - 10, The engines block currently only requires Node >=24; add a pnpm constraint to prevent older pnpm clients from using the new workspace format by updating the "engines" object to include "pnpm": ">=11" (aligning with the existing packageManager pin to pnpm@11.1.2). Edit the package.json "engines" field so it contains both "node": ">=24" and "pnpm": ">=11" to ensure the repo rejects older pnpm versions that don’t support the v11 workspace syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/indy-charts/tsconfig.json`:
- Line 8: The tsconfig setting "ignoreDeprecations": "6.0" is invalid for the
client workspace's TypeScript (~5.9.3); update either the client TypeScript
dependency to 6.0+ in client/package.json or change the tsconfig value to a
value supported by both versions (e.g., set "ignoreDeprecations" to "5.0") so
that the compiler in the client workspace no longer rejects the tsconfig; ensure
you modify the entry for "ignoreDeprecations" in libs/indy-charts/tsconfig.json
accordingly and run a local build to confirm the error is resolved.
In `@README.md`:
- Line 21: Update the pnpm version wording in the README by replacing the string
"[pnpm](https://pnpm.io/) (v11 LTS or later) - Installed via platform package
managers:" with wording that does not reference an LTS track (e.g.,
"[pnpm](https://pnpm.io/) (v11 or later) - Installed via platform package
managers:") so the README accurately reflects pnpm's versioning model.
---
Nitpick comments:
In `@package.json`:
- Around line 8-10: The engines block currently only requires Node >=24; add a
pnpm constraint to prevent older pnpm clients from using the new workspace
format by updating the "engines" object to include "pnpm": ">=11" (aligning with
the existing packageManager pin to pnpm@11.1.2). Edit the package.json "engines"
field so it contains both "node": ">=24" and "pnpm": ">=11" to ensure the repo
rejects older pnpm versions that don’t support the v11 workspace syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6be6f900-d1ab-46e3-b49c-b01f52e2d38a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (24)
.claude/agents.claude/skills.editorconfig.vscode/tasks.jsonAGENTS.mdREADME.mdclient/package.jsondocs/CONTRIBUTING.mddocs/plans/reusable-charts.plan.mdlibs/chartjs-financial/.npmignorelibs/chartjs-financial/AGENTS.mdlibs/chartjs-financial/README.mdlibs/chartjs-financial/package.jsonlibs/indy-charts/.npmignorelibs/indy-charts/AGENTS.mdlibs/indy-charts/README.mdlibs/indy-charts/package.jsonlibs/indy-charts/tsconfig.jsonlibs/indy-charts/tsup.config.tspackage.jsonpnpm-workspace.yamlscripts/smoke-indy-charts-vitepress-consumer.jstests/playwright/package.jsontests/vitepress/package.json
💤 Files with no reviewable changes (2)
- .claude/agents
- .claude/skills
DaveSkender
changed the title
indy-charts
Contributor
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)
libs/indy-charts/package.json (1)
20-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNode.js version requirement inconsistent with documentation.
The package specifies
"node": ">=22"but the installation guide (tests/vitepress/guide/installation.md:5) states "Node.js 24+". These should be aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/indy-charts/package.json` at line 20, The engines.node field in package.json is set to ">=22" but the installation guide requires Node.js 24+; update the engines entry in libs/indy-charts package.json (the "node" key) to ">=24" to match tests/vitepress/guide/installation.md, or alternatively update the guide to "Node.js 22+"—pick one and make both artifacts consistent so the "node" engine in package.json and the installation.md statement align.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/vitepress/guide/installation.md`:
- Line 5: Update the documentation line that currently reads "Make sure you have
Node.js 24+ installed." to instead state "Make sure you have Node.js 22+
installed." so it matches the package.json engine requirement (">=22"); locate
and replace that exact string in tests/vitepress/guide/installation.md to keep
docs and package.json consistent.
---
Outside diff comments:
In `@libs/indy-charts/package.json`:
- Line 20: The engines.node field in package.json is set to ">=22" but the
installation guide requires Node.js 24+; update the engines entry in
libs/indy-charts package.json (the "node" key) to ">=24" to match
tests/vitepress/guide/installation.md, or alternatively update the guide to
"Node.js 22+"—pick one and make both artifacts consistent so the "node" engine
in package.json and the installation.md statement align.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a4a1740-d016-49c0-8694-34618ba765f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (11)
libs/indy-charts/package.jsontests/vitepress/AGENTS.mdtests/vitepress/README.mdtests/vitepress/examples/index.mdtests/vitepress/examples/indicators.mdtests/vitepress/examples/multiple.mdtests/vitepress/guide/api-client.mdtests/vitepress/guide/index.mdtests/vitepress/guide/installation.mdtests/vitepress/guide/quick-start.mdtests/vitepress/index.md
✅ Files skipped from review due to trivial changes (6)
- tests/vitepress/index.md
- tests/vitepress/guide/index.md
- tests/vitepress/AGENTS.md
- tests/vitepress/README.md
- tests/vitepress/examples/index.md
- tests/vitepress/examples/indicators.md
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/vitepress/guide/api-client.md
- tests/vitepress/examples/multiple.md
- tests/vitepress/guide/quick-start.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/Functions/UpdateQuotes.cs (1)
160-193:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPropagate
CancellationTokenthrough the quote update I/O path.In this touched path, external I/O calls are awaited without a cancellation token. Add a token parameter to
Run(available in Azure Functions isolated worker model) and thread it throughStoreQuoteDailytoListHistoricalBarsAsync, which supports cancellation. This enables graceful shutdown during long-running API operations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/Functions/UpdateQuotes.cs` around lines 160 - 193, Update the I/O path to accept and propagate a CancellationToken: add a CancellationToken parameter to Run and forward it into StoreQuoteDaily (e.g., StoreQuoteDaily(string symbol, CancellationToken ct)), then pass that token into async I/O calls that accept it—specifically into alpacaDataClient.ListHistoricalBarsAsync(...) and any storage method like _storage.PutBlobAsync(...) if it supports a CancellationToken; also honor the token by passing it into any awaits and by throwing/returning early on ct.IsCancellationRequested. Ensure all modified calls reference the new parameter names (e.g., ct) so cancellation is threaded end-to-end.
🧹 Nitpick comments (1)
scripts/generate-backup-indicators.js (1)
45-47: ⚡ Quick winAdd a response shape guard before endpoint normalization.
Line 46 assumes an array payload; when the API shape drifts, this fails late and can silently retain stale data. Fail fast with a clear error before calling
normalizeEndpoints.Based on learnings: Implement explicit error handling with try/catch blocks or Result types; do not remove error handling to simplify code.Proposed patch
- const raw = await fetchListings(); - const listings = normalizeEndpoints(raw); + const raw = await fetchListings(); + if (!Array.isArray(raw)) { + throw new Error("Expected /indicators to return an array"); + } + const listings = normalizeEndpoints(raw);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate-backup-indicators.js` around lines 45 - 47, The code calls normalizeEndpoints(raw) assuming fetchListings() returns an array; add an explicit response-shape guard and error handling: after calling fetchListings() check that raw is the expected array/object shape (e.g., Array.isArray(raw) or validate required keys) and throw or return a descriptive error (including the raw response) if the shape is wrong, then call normalizeEndpoints(raw); additionally wrap the fetch + normalization in a try/catch so fetchListings and normalizeEndpoints errors are caught and rethrown or logged with context (reference fetchListings and normalizeEndpoints) to fail fast and avoid silently keeping stale data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/app/services/api.service.spec.ts`:
- Around line 42-49: The test mock in api.service.spec.ts is using a Date object
where the RawQuote interface defines timestamp as a string; change the mocked
quote's timestamp to a raw ISO string (e.g. "2023-01-01T00:00:00.000Z" or
"2023-01-01") so the service's string-to-Date parsing/normalization is
exercised. Update the mock object used in the spec (the RawQuote test fixture)
and any assertions that assumed a Date so they verify the normalized output from
the code under test instead.
In `@client/src/app/services/api.service.ts`:
- Around line 96-100: The toQuotes method currently does new Date(q.timestamp)
without validation; update toQuotes in client/src/app/services/api.service.ts to
validate timestamps like libs/indy-charts/api/client.ts does: call the shared
parseQuoteDate (or duplicate its logic) for each RawQuote.timestamp, throw or
return a clear error when parseQuoteDate reports an invalid date, and ensure the
mapped Quote.timestamp is a Date instance only when valid; reference the
toQuotes function and parseQuoteDate to locate where to apply this change.
In `@server/WebApi/Endpoints.cs`:
- Around line 54-55: The endpoint signatures (e.g., GetAdl) no longer accept the
parameters advertised by the indicator metadata (such as SMA/signal variant
controls), causing client/UI parameters to be ignored; update each affected
endpoint method (e.g., GetAdl, and the other endpoints at the same pattern
around lines with GetX methods) to accept the full parameter set required by the
metadata and pass them through into the handler call (the Get(...) invocation)
so the UI controls are applied — locate the endpoint methods that currently use
the shorthand expression-bodied form (like public Task<IActionResult> GetAdl()
=> Get(quotes => quotes.ToAdl());) and restore explicit parameter lists and
forwarding to the underlying processing (e.g., include variant/sma/ signal
parameters and feed them into ToAdl or the delegate used by Get).
- Around line 36-41: The Get<T> method and upstream quote fetch need to accept
and pass a CancellationToken so HTTP request cancellations stop backend work:
add a CancellationToken parameter to the controller actions, change
Get<T>(Func<IReadOnlyList<Quote>, IEnumerable<T>> indicatorFunc) to
Get<T>(Func<IReadOnlyList<Quote>, IEnumerable<T>> indicatorFunc,
CancellationToken cancellationToken), update calls to quoteFeed.Get(...) /
IQuoteService.Get(...) to accept and propagate that token, await
quoteFeed.Get(cancellationToken) and thread the same token through all callers
(controller action → Get<T> → IQuoteService.Get/quoteFeed.Get), and ensure any
long-running loops check cancellationToken.ThrowIfCancellationRequested() where
appropriate.
---
Outside diff comments:
In `@server/Functions/UpdateQuotes.cs`:
- Around line 160-193: Update the I/O path to accept and propagate a
CancellationToken: add a CancellationToken parameter to Run and forward it into
StoreQuoteDaily (e.g., StoreQuoteDaily(string symbol, CancellationToken ct)),
then pass that token into async I/O calls that accept it—specifically into
alpacaDataClient.ListHistoricalBarsAsync(...) and any storage method like
_storage.PutBlobAsync(...) if it supports a CancellationToken; also honor the
token by passing it into any awaits and by throwing/returning early on
ct.IsCancellationRequested. Ensure all modified calls reference the new
parameter names (e.g., ct) so cancellation is threaded end-to-end.
---
Nitpick comments:
In `@scripts/generate-backup-indicators.js`:
- Around line 45-47: The code calls normalizeEndpoints(raw) assuming
fetchListings() returns an array; add an explicit response-shape guard and error
handling: after calling fetchListings() check that raw is the expected
array/object shape (e.g., Array.isArray(raw) or validate required keys) and
throw or return a descriptive error (including the raw response) if the shape is
wrong, then call normalizeEndpoints(raw); additionally wrap the fetch +
normalization in a try/catch so fetchListings and normalizeEndpoints errors are
caught and rethrown or logged with context (reference fetchListings and
normalizeEndpoints) to fail fast and avoid silently keeping stale data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc7d133f-71bf-4d0a-aec3-29ed718b0760
📒 Files selected for processing (36)
.vscode/tasks.jsonclient/src/app/data/backup-indicators.jsonclient/src/app/data/backup-quotes.jsonclient/src/app/data/backup-quotes.tsclient/src/app/services/api.service.spec.tsclient/src/app/services/api.service.tsclient/src/app/services/chart.service.spec.tsdocs/plans/stock-indicators-v3-upgrade.mdlibs/chartjs-financial/datasets.spec.tslibs/chartjs-financial/datasets.tslibs/indy-charts/api/client.spec.tslibs/indy-charts/api/client.tslibs/indy-charts/api/static.spec.tslibs/indy-charts/api/static.tslibs/indy-charts/charts/chart-manager.spec.tslibs/indy-charts/config/types.tslibs/indy-charts/data/transformers.spec.tslibs/indy-charts/data/transformers.tslibs/indy-charts/vue/stock-indicator-chart.spec.tsscripts/generate-backup-indicators.jsserver/Directory.Packages.propsserver/Functions/UpdateQuotes.csserver/Functions/packages.lock.jsonserver/WebApi.Tests/Services/RandomQuotesTests.csserver/WebApi.Tests/Services/UtilitiesTests.csserver/WebApi.Tests/packages.lock.jsonserver/WebApi/Endpoints.csserver/WebApi/GlobalUsings.csserver/WebApi/Services/Service.Metadata.csserver/WebApi/Services/Service.Quotes.Failover.csserver/WebApi/Services/Service.Quotes.Random.csserver/WebApi/Services/Service.Quotes.csserver/WebApi/packages.lock.jsontests/playwright/playwright.config.tstests/playwright/vitepress.spec.tstests/vitepress/package.json
💤 Files with no reviewable changes (1)
- docs/plans/stock-indicators-v3-upgrade.md
✅ Files skipped from review due to trivial changes (2)
- server/Directory.Packages.props
- client/src/app/services/chart.service.spec.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- .vscode/tasks.json
- libs/indy-charts/vue/stock-indicator-chart.spec.ts
- server/WebApi.Tests/packages.lock.json
- server/WebApi/packages.lock.json
- server/Functions/packages.lock.json
Contributor
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)
server/WebApi/Services/Service.Quotes.cs (1)
65-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not swallow cancellation exceptions as fallback success.
Line 65 catches
Exception, which also catchesOperationCanceledException. Since theCancellationTokenis passed to async operations on lines 38, 44, and 53, cancellation can be requested by the caller. When cancellation occurs, this catch-all converts the cancellation exception into a successful response with backup data, masking the cancellation semantic and violating graceful cancellation support required by the coding guidelines.Add a separate handler for
OperationCanceledExceptionbefore the general exception handler to allow proper cancellation propagation:Suggested fix
+ catch (OperationCanceledException) + { + throw; + } // failover to backup quotes for local development and testing catch (Exception ex) { LogRetrieveQuotesFailed(ex, symbol); return QuoteBackup.BackupQuotes; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/WebApi/Services/Service.Quotes.cs` around lines 65 - 69, The catch-all handler currently catching Exception in Service.Quotes.cs (the block that calls LogRetrieveQuotesFailed(ex, symbol) and returns QuoteBackup.BackupQuotes) also swallows OperationCanceledException; add a specific catch (OperationCanceledException) before the general catch to rethrow/propagate cancellation (e.g., catch (OperationCanceledException) { throw; }) so cancellation is not converted into a successful fallback, then keep the existing catch (Exception ex) block for non-cancellation errors that log via LogRetrieveQuotesFailed and return QuoteBackup.BackupQuotes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/WebApi/Services/Service.Storage.cs`:
- Around line 44-47: The PutBlobAsync entrypoint should validate its inputs
before using them: call ArgumentNullException.ThrowIfNull(blobName) and
ArgumentNullException.ThrowIfNull(content), and also guard against
empty/whitespace blobName (e.g., throw new ArgumentException for
string.IsNullOrWhiteSpace(blobName)); do not proceed to create the MemoryStream
or call _blobClient.GetBlobClient(blobName).UploadAsync(...) if validation
fails. Keep these checks at the start of the PutBlobAsync method to satisfy the
project coding guideline for explicit argument validation.
---
Outside diff comments:
In `@server/WebApi/Services/Service.Quotes.cs`:
- Around line 65-69: The catch-all handler currently catching Exception in
Service.Quotes.cs (the block that calls LogRetrieveQuotesFailed(ex, symbol) and
returns QuoteBackup.BackupQuotes) also swallows OperationCanceledException; add
a specific catch (OperationCanceledException) before the general catch to
rethrow/propagate cancellation (e.g., catch (OperationCanceledException) {
throw; }) so cancellation is not converted into a successful fallback, then keep
the existing catch (Exception ex) block for non-cancellation errors that log via
LogRetrieveQuotesFailed and return QuoteBackup.BackupQuotes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2daf559-59e3-49af-be0c-9ea8cac3bff5
📒 Files selected for processing (17)
docs/plans/missing-indicators-implementation.mdlibs/indy-charts/config/annotations.tslibs/indy-charts/config/common.tsscripts/smoke-indy-charts-vitepress-consumer.jsserver/Functions/UpdateQuotes.csserver/WebApi/Endpoints.csserver/WebApi/Services/Service.Quotes.csserver/WebApi/Services/Service.Storage.cstests/playwright/vitepress.spec.tstests/vitepress/.env.exampletests/vitepress/.gitignoretests/vitepress/.vitepress/config.tstests/vitepress/.vitepress/theme/custom.csstests/vitepress/.vitepress/theme/index.tstests/vitepress/examples/StaticChart.vuetests/vitepress/examples/custom-data.mdtests/vitepress/examples/sample-quotes.ts
💤 Files with no reviewable changes (1)
- docs/plans/missing-indicators-implementation.md
✅ Files skipped from review due to trivial changes (4)
- libs/indy-charts/config/annotations.ts
- tests/vitepress/.env.example
- tests/vitepress/examples/sample-quotes.ts
- tests/vitepress/.gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/vitepress/.vitepress/config.ts
- tests/playwright/vitepress.spec.ts
- scripts/smoke-indy-charts-vitepress-consumer.js
- server/WebApi/Endpoints.cs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/WebApi/Services/Service.Quotes.cs (1)
30-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCatch and re-throw
OperationCanceledExceptionbefore broad exception handler.At line 65, the
catch (Exception ex)block will interceptOperationCanceledExceptionthrown by the newly tokenized calls (ExistsAsync,DownloadAsync,DeserializeAsync), returning backup quotes instead of propagating cancellation. This breaks cancellation semantics.Proposed fix
- catch (Exception ex) + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) { LogRetrieveQuotesFailed(ex, symbol); return QuoteBackup.BackupQuotes; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/WebApi/Services/Service.Quotes.cs` around lines 30 - 68, In Get (Service.Quotes.cs) ensure OperationCanceledException is not swallowed: add a specific catch for OperationCanceledException and rethrow it immediately (before the existing catch (Exception ex)) so tokenized calls (ExistsAsync, DownloadAsync, DeserializeAsync) can propagate cancellation; keep the existing broad catch to log via LogRetrieveQuotesFailed(ex, symbol) and return QuoteBackup.BackupQuotes for non-cancellation exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/WebApi/Services/Service.Quotes.cs`:
- Around line 30-68: In Get (Service.Quotes.cs) ensure
OperationCanceledException is not swallowed: add a specific catch for
OperationCanceledException and rethrow it immediately (before the existing catch
(Exception ex)) so tokenized calls (ExistsAsync, DownloadAsync,
DeserializeAsync) can propagate cancellation; keep the existing broad catch to
log via LogRetrieveQuotesFailed(ex, symbol) and return QuoteBackup.BackupQuotes
for non-cancellation exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c63270c2-b4ae-4af4-ab66-fad68d4f2ed0
📒 Files selected for processing (25)
client/src/app/data/backup-indicators.jsonclient/src/app/services/api.service.spec.tsclient/src/app/services/api.service.tsdocs/plans/missing-indicators-implementation.mddocs/plans/reusable-charts.plan.mdlibs/indy-charts/config/annotations.tslibs/indy-charts/config/common.tsscripts/smoke-indy-charts-vitepress-consumer.jsserver/Functions/UpdateQuotes.csserver/WebApi/Endpoints.csserver/WebApi/Models/IndicatorListing.csserver/WebApi/Services/Service.Metadata.csserver/WebApi/Services/Service.Quotes.csserver/WebApi/Services/Service.Storage.cstests/playwright/vitepress.spec.tstests/vitepress/.env.exampletests/vitepress/.gitignoretests/vitepress/.vitepress/config.tstests/vitepress/.vitepress/theme/custom.csstests/vitepress/.vitepress/theme/index.tstests/vitepress/env.d.tstests/vitepress/examples/StaticChart.vuetests/vitepress/examples/custom-data.mdtests/vitepress/examples/sample-quotes.tstests/vitepress/tsconfig.json
💤 Files with no reviewable changes (1)
- docs/plans/missing-indicators-implementation.md
✅ Files skipped from review due to trivial changes (7)
- tests/vitepress/.gitignore
- tests/vitepress/examples/sample-quotes.ts
- tests/vitepress/tsconfig.json
- tests/vitepress/.env.example
- tests/vitepress/env.d.ts
- tests/vitepress/.vitepress/config.ts
- docs/plans/reusable-charts.plan.md
🚧 Files skipped from review as they are similar to previous changes (11)
- libs/indy-charts/config/annotations.ts
- libs/indy-charts/config/common.ts
- server/Functions/UpdateQuotes.cs
- tests/vitepress/.vitepress/theme/custom.css
- client/src/app/services/api.service.spec.ts
- tests/vitepress/examples/custom-data.md
- tests/vitepress/.vitepress/theme/index.ts
- scripts/smoke-indy-charts-vitepress-consumer.js
- tests/playwright/vitepress.spec.ts
- tests/vitepress/examples/StaticChart.vue
- server/WebApi/Endpoints.cs
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/WebApi/Services/Service.Quotes.cs`:
- Around line 30-33: In Get(string symbol, CancellationToken ct = default)
validate the input before building blobName: throw ArgumentNullException if
symbol is null or whitespace, and throw ArgumentException if symbol is not one
of the allowed values ("SPY" or "QQQ"); do this at the start of the Get method
(before computing blobName) so invalid inputs fail fast and callers get clear
exceptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 639f61d7-2b6e-4113-8098-97c51ce7bc38
📒 Files selected for processing (1)
server/WebApi/Services/Service.Quotes.cs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
libs/indy-charts/config/types.ts (1)
26-29: ⚡ Quick winRequire at least one time key at the type level.
With both
timestamp?anddate?optional, callers can create rows with neither field, pushing contract errors to runtime.Proposed refactor
+type RawTimeField = + | { timestamp: string; date?: string } + | { timestamp?: string; date: string }; + -export interface RawQuote { - /** Skender.Stock.Indicators v3+ field name */ - timestamp?: string; - /** `@deprecated` Skender v2 field name — accepted for backward compatibility */ - date?: string; +export type RawQuote = RawTimeField & { open: number; high: number; low: number; close: number; volume: number; -} +}; -export interface IndicatorDataRow { - /** Skender.Stock.Indicators v3+ field name */ - timestamp?: string; - /** `@deprecated` Skender v2 field name — accepted for backward compatibility */ - date?: string; +export type IndicatorDataRow = RawTimeField & { candle: Quote; [key: string]: unknown; // For dynamic indicator result values -} +};Also applies to: 38-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/indy-charts/config/types.ts` around lines 26 - 29, The timestamp and date properties are both optional allowing objects with neither key; update the type(s) that declare timestamp?: string and date?: string to require at least one of them (e.g., replace the two optional fields with a type-level "at least one of" constraint or a small union/discriminated union so callers must supply either timestamp or date). Ensure you modify both occurrences (the block with timestamp/date at lines ~26-29 and the similar block at ~38-41) and keep the property names timestamp and date unchanged for backward compatibility.client/src/test-utils/chart-config-serializer.ts (1)
46-57: ⚡ Quick win
[Function]normalization is currently unreachable.Functions are removed by
JSON.stringifybeforenormalizeValueruns, so this branch never executes.Proposed fix
- const sanitized: ChartSerializable = JSON.parse(JSON.stringify(val)) as ChartSerializable; + const sanitized: ChartSerializable = val;Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/test-utils/chart-config-serializer.ts` around lines 46 - 57, normalizeValue's function branch is never hit because functions get stripped by JSON.stringify before normalizeValue runs; update the serialization flow to run normalization prior to stringification (or use JSON.stringify's replacer to call normalizeValue) so the typeof value === "function" case in normalizeValue can execute. Adjust the code that calls normalizeValue (and the related recurse) so normalization occurs on the original object tree (referencing normalizeValue, isIdKey, recurse) rather than after JSON.stringify; apply the same change to the similar logic referenced at the other occurrence (the branch noted around line 65).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/indy-charts/api/client.ts`:
- Line 82: The current call parseQuoteDate(q.timestamp ?? q.date ?? "", index)
uses nullish coalescing so an empty string in q.timestamp will not fall back to
q.date; change it to pick the first non-empty string before parsing (e.g., use a
logical-OR or an explicit check to prefer q.timestamp when it has content,
otherwise q.date, otherwise empty) and pass that result to parseQuoteDate;
update the invocation in the function where parseQuoteDate is called
(referencing q.timestamp, q.date, parseQuoteDate, and index) accordingly.
---
Nitpick comments:
In `@client/src/test-utils/chart-config-serializer.ts`:
- Around line 46-57: normalizeValue's function branch is never hit because
functions get stripped by JSON.stringify before normalizeValue runs; update the
serialization flow to run normalization prior to stringification (or use
JSON.stringify's replacer to call normalizeValue) so the typeof value ===
"function" case in normalizeValue can execute. Adjust the code that calls
normalizeValue (and the related recurse) so normalization occurs on the original
object tree (referencing normalizeValue, isIdKey, recurse) rather than after
JSON.stringify; apply the same change to the similar logic referenced at the
other occurrence (the branch noted around line 65).
In `@libs/indy-charts/config/types.ts`:
- Around line 26-29: The timestamp and date properties are both optional
allowing objects with neither key; update the type(s) that declare timestamp?:
string and date?: string to require at least one of them (e.g., replace the two
optional fields with a type-level "at least one of" constraint or a small
union/discriminated union so callers must supply either timestamp or date).
Ensure you modify both occurrences (the block with timestamp/date at lines
~26-29 and the similar block at ~38-41) and keep the property names timestamp
and date unchanged for backward compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd4effe6-7fc0-4012-8b87-8df0d3d88e54
📒 Files selected for processing (10)
client/src/app/services/api.service.tsclient/src/test-utils/chart-config-serializer.tslibs/indy-charts/api/client.tslibs/indy-charts/api/static.tslibs/indy-charts/config/types.tslibs/indy-charts/data/transformers.tsserver/WebApi/Services/Service.Quotes.Random.csserver/WebApi/Services/Service.Quotes.csserver/WebApi/Services/Service.Storage.csserver/WebApi/appsettings.json
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/indy-charts/api/static.ts
- server/WebApi/appsettings.json
- libs/indy-charts/data/transformers.ts
- client/src/app/services/api.service.ts
- server/WebApi/Services/Service.Quotes.cs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
…; update Vitest configuration and package dependencies
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
server/WebApi.Tests/Endpoints/MainEndpointsTests.cs (1)
46-48: ⚡ Quick winAdd assertions for
CancellationTokenforwarding.All quote-service setups use
It.IsAny<CancellationToken>(), so these tests won’t catch regressions where controller endpoints stop passingHttpContext.RequestAborted. Add at least one verification per endpoint family for the exact token.Also applies to: 93-95, 114-116, 135-137, 156-158, 177-179, 198-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/WebApi.Tests/Endpoints/MainEndpointsTests.cs` around lines 46 - 48, Tests currently use It.IsAny<CancellationToken>() when setting up _quoteServiceMock.Get, so they don't detect if the controller stops forwarding HttpContext.RequestAborted; update each endpoint-family test (the setups around _quoteServiceMock.Setup(q => q.Get(...)) for the various endpoints) to assert the exact token is forwarded by either: 1) changing the Setup to accept It.Is<CancellationToken>(ct => ct == expectedToken) where expectedToken is the controller's HttpContext.RequestAborted captured from the test HttpContext, or 2) keep the flexible Setup but add a corresponding _quoteServiceMock.Verify(q => q.Get(It.Is<CancellationToken>(ct => ct == expectedToken)), Times.Once()) after invoking the controller endpoint; reference the _quoteServiceMock and Get(...) invocation in each endpoint-family test to add this verification.client/src/app/services/utility.service.spec.ts (1)
72-73: ⚡ Quick winReplace
anymocks with typed mocks.
metaMock,titleMock, and(service as any)bypass type safety in a file under strict TypeScript guidelines. Use narrowed interfaces/typed partials for these test doubles.As per coding guidelines, "
client/src/**/*.ts: Enable TypeScript strict mode and maintain comprehensive type safety; avoid@ts-ignore, any types, and strict mode disabling."Also applies to: 87-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/app/services/utility.service.spec.ts` around lines 72 - 73, Replace the loose any types by typed test doubles: declare metaMock as jasmine.SpyObj<Meta> and titleMock as jasmine.SpyObj<Title> (create them with jasmine.createSpyObj('Meta', ['addTag','getTag','updateTag']) and jasmine.createSpyObj('Title', ['setTitle','getTitle'])), and replace usages of (service as any) with a properly-typed reference (either declare service: UtilityService or obtain it via TestBed.inject(UtilityService)); update expectations to use the spy methods (e.g., metaMock.addTag.calls...) so TypeScript strict mode and typings for Meta/Title and UtilityService are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/app/services/utility.service.spec.ts`:
- Around line 3-4: The test file imports an unused Vitest alias and declares
unused spy variables causing CI warnings; remove the redundant import alias `vi
as vitest` from the imports and either remove or use the spy variables
`metaServiceMock` and `titleServiceMock` (declared in the spec) — if they are
unnecessary delete their assignments, otherwise add assertions against them
(e.g., expect(metaServiceMock).toHaveBeenCalled...) so they are referenced;
update any related tests that reference `Mock`/`vi` to use the existing `vi`
import if needed and ensure no unused bindings remain.
- Line 51: The failing linter flags unnecessary escapes in the regexes used in
the test assertions; update the three toMatch regexes in utility.service.spec.ts
(the expect(result).toMatch(...) assertions) by moving the hyphen out of the
middle of the character class so it is not escaped (e.g. change
/^chart[0-9a-f\-]{36}$/i to /^chart[0-9a-f-]{36}$/i), and apply the same fix to
the other two toMatch occurrences to satisfy no-useless-escape.
---
Nitpick comments:
In `@client/src/app/services/utility.service.spec.ts`:
- Around line 72-73: Replace the loose any types by typed test doubles: declare
metaMock as jasmine.SpyObj<Meta> and titleMock as jasmine.SpyObj<Title> (create
them with jasmine.createSpyObj('Meta', ['addTag','getTag','updateTag']) and
jasmine.createSpyObj('Title', ['setTitle','getTitle'])), and replace usages of
(service as any) with a properly-typed reference (either declare service:
UtilityService or obtain it via TestBed.inject(UtilityService)); update
expectations to use the spy methods (e.g., metaMock.addTag.calls...) so
TypeScript strict mode and typings for Meta/Title and UtilityService are
preserved.
In `@server/WebApi.Tests/Endpoints/MainEndpointsTests.cs`:
- Around line 46-48: Tests currently use It.IsAny<CancellationToken>() when
setting up _quoteServiceMock.Get, so they don't detect if the controller stops
forwarding HttpContext.RequestAborted; update each endpoint-family test (the
setups around _quoteServiceMock.Setup(q => q.Get(...)) for the various
endpoints) to assert the exact token is forwarded by either: 1) changing the
Setup to accept It.Is<CancellationToken>(ct => ct == expectedToken) where
expectedToken is the controller's HttpContext.RequestAborted captured from the
test HttpContext, or 2) keep the flexible Setup but add a corresponding
_quoteServiceMock.Verify(q => q.Get(It.Is<CancellationToken>(ct => ct ==
expectedToken)), Times.Once()) after invoking the controller endpoint; reference
the _quoteServiceMock and Get(...) invocation in each endpoint-family test to
add this verification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4cb669f4-f65e-42f9-9bed-698dfae0f399
📒 Files selected for processing (7)
client/src/app/app.component.spec.tsclient/src/app/services/utility.service.spec.tsclient/vitest.config.tsserver/Directory.Packages.propsserver/WebApi.Tests/Endpoints/MainEndpointsTests.csserver/WebApi.Tests/WebApi.Tests.csprojserver/WebApi.Tests/packages.lock.json
✅ Files skipped from review due to trivial changes (2)
- server/Directory.Packages.props
- server/WebApi.Tests/packages.lock.json
… tests and documentation
…nd improved rendering logic
…d supporting multiple API origins
Contributor
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)
libs/indy-charts/charts/overlay-chart.ts (1)
156-159:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
onResizecallback when rebuilding options inupdateTheme().The
render()method sets anonResizecallback for legend re-anchoring (line 58), butupdateTheme()replaceschart.optionsentirely (lines 156–159), causing the callback to be lost. After a theme update, resize events will no longer re-anchor legend annotations.💡 Suggested fix
- this.chart.options = buildFinancialChartOptions( - baseOverlayOptions(this.volumeAxisSize, settings) - ); + const nextOptions = buildFinancialChartOptions( + baseOverlayOptions(this.volumeAxisSize, settings) + ); + nextOptions.onResize = () => { + if (this._latestLegendSelections.length > 0) { + queueMicrotask(() => { + this._applyLegendAnnotations(); + }); + } + }; + this.chart.options = nextOptions;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/indy-charts/charts/overlay-chart.ts` around lines 156 - 159, updateTheme() replaces this.chart.options with a fresh object from buildFinancialChartOptions(baseOverlayOptions(...)), which discards the previously set onResize callback that render() attaches for legend re-anchoring; modify updateTheme() to preserve (or reattach) the existing onResize handler by reading the current this.chart.options.onResize (or the legend re-anchor function) and assigning it onto the new options object after calling buildFinancialChartOptions(baseOverlayOptions(this.volumeAxisSize, settings)), or explicitly call the same onResize re-anchor routine used in render() and set newOptions.onResize = oldOnResize before assigning this.chart.options and calling this.chart.update("none").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/app/services/utility.service.spec.ts`:
- Around line 68-69: Replace the untyped `any` mocks and ad-hoc property casting
by creating properly typed spy objects for Angular's Title and Meta and
injecting them via TestBed: declare titleMock as jasmine.SpyObj<Title> =
jasmine.createSpyObj('Title', ['setTitle']) and metaMock as jasmine.SpyObj<Meta>
= jasmine.createSpyObj('Meta', ['addTag','updateTag']), then provide them in
TestBed.configureTestingModule({ providers: [{ provide: Title, useValue:
titleMock }, { provide: Meta, useValue: metaMock }] }) so UtilityService (or the
instance created in tests) receives the mocks without using `any` or casting; if
you must access internal properties on UtilityService, use a narrow type
assertion (e.g., as unknown as { title: Title }) or better expose a
setter/constructor injection rather than casting to `any`.
---
Outside diff comments:
In `@libs/indy-charts/charts/overlay-chart.ts`:
- Around line 156-159: updateTheme() replaces this.chart.options with a fresh
object from buildFinancialChartOptions(baseOverlayOptions(...)), which discards
the previously set onResize callback that render() attaches for legend
re-anchoring; modify updateTheme() to preserve (or reattach) the existing
onResize handler by reading the current this.chart.options.onResize (or the
legend re-anchor function) and assigning it onto the new options object after
calling buildFinancialChartOptions(baseOverlayOptions(this.volumeAxisSize,
settings)), or explicitly call the same onResize re-anchor routine used in
render() and set newOptions.onResize = oldOnResize before assigning
this.chart.options and calling this.chart.update("none").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d0761ef-4e3e-4e4b-b0aa-ddc52e96dbae
📒 Files selected for processing (27)
client/src/app/app.component.spec.tsclient/src/app/data/backup-quotes.tsclient/src/app/services/utility.service.spec.tsclient/vitest.config.tslibs/indy-charts/README.mdlibs/indy-charts/api/client.tslibs/indy-charts/charts/chart-manager.spec.tslibs/indy-charts/charts/chart-manager.tslibs/indy-charts/charts/overlay-chart.tslibs/indy-charts/config/annotations.tslibs/indy-charts/config/index.tslibs/indy-charts/config/theme-colors.tslibs/indy-charts/index.tslibs/indy-charts/package.jsonpackage.jsonserver/Directory.Packages.propsserver/WebApi.Tests/Endpoints/MainEndpointsTests.csserver/WebApi.Tests/WebApi.Tests.csprojserver/WebApi.Tests/packages.lock.jsontests/playwright/vitepress.spec.tstests/vitepress/.vitepress/config.tstests/vitepress/README.mdtests/vitepress/examples/index.mdtests/vitepress/examples/indicators.mdtests/vitepress/guide/installation.mdtests/vitepress/guide/quick-start.mdtests/vitepress/guide/themes.md
✅ Files skipped from review due to trivial changes (7)
- libs/indy-charts/config/index.ts
- libs/indy-charts/config/theme-colors.ts
- tests/vitepress/guide/themes.md
- tests/vitepress/README.md
- tests/vitepress/guide/installation.md
- tests/vitepress/guide/quick-start.md
- tests/vitepress/.vitepress/config.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/vitepress/examples/indicators.md
- server/Directory.Packages.props
- libs/indy-charts/api/client.ts
- server/WebApi.Tests/WebApi.Tests.csproj
- libs/indy-charts/README.md
- client/vitest.config.ts
- tests/playwright/vitepress.spec.ts
- client/src/app/app.component.spec.ts
- server/WebApi.Tests/Endpoints/MainEndpointsTests.cs
- server/WebApi.Tests/packages.lock.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.