refactor: removes intermediate layers to speed up bid matching#3172
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors provider-inventory from StreamStatusMessage/Inventory (number-based) to ClusterState/NodeState with ResourcePair bigints: adds quantity parsing, stream mapping, persistence as JSONB bigint-aware, hydration into ResourcePair, service/matcher inputs adjusted, equality and rollup logic rewritten, and tests converted to the new model. ChangesInventory model & ingestion consolidation (ClusterState, ResourcePair, parsing, persistence, services, matcher)
ESLint override for decorators (small, unrelated)
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3172 +/- ##
==========================================
- Coverage 63.63% 62.82% -0.81%
==========================================
Files 1085 1044 -41
Lines 26248 25281 -967
Branches 6363 6221 -142
==========================================
- Hits 16703 15883 -820
+ Misses 8357 8219 -138
+ Partials 1188 1179 -9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![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: 4
🧹 Nitpick comments (4)
apps/provider-inventory/src/providers/chain-sdk.provider.ts (1)
6-6: ⚡ Quick winUse
@src/*alias for intra-app source imports.Line 6 should use the backend alias to stay consistent with repository import rules.
Suggested diff
-import { APP_CONFIG } from "./app-config.provider"; +import { APP_CONFIG } from "@src/providers/app-config.provider";As per coding guidelines
apps/**/*.ts: "Use path aliases@src/*for source files and@test/*for test files in backend applications".🤖 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 `@apps/provider-inventory/src/providers/chain-sdk.provider.ts` at line 6, Replace the relative import for the APP_CONFIG provider with the backend path alias: find the import statement importing APP_CONFIG (in chain-sdk.provider.ts) and change it from a relative path ("./app-config.provider") to the `@src/`* alias equivalent so the file uses the repository-wide backend alias (e.g., import APP_CONFIG from the aliased module) to comply with the apps/** import rule.apps/provider-inventory/src/types/inventory.ts (1)
1-1: ⚡ Quick winUse
@src/*alias for internal source imports.This relative import should use the backend source alias to stay consistent with repo rules.
Proposed change
-import type { ClusterState } from "./inventory.types"; +import type { ClusterState } from "@src/types/inventory.types";As per coding guidelines,
apps/**/*.ts: Use path aliases@src/*for source files and@test/*for test files in backend applications.🤖 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 `@apps/provider-inventory/src/types/inventory.ts` at line 1, The import line using a relative path ("import type { ClusterState } from \"./inventory.types\";") should use the backend source path alias instead; update the import to reference the aliased module (e.g., import type { ClusterState } from "@src/types/inventory.types" or the correct `@src/`* path that maps to this file) so it uses the `@src/`* alias convention, and verify the referenced symbol ClusterState and the module name match the aliased location.apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts (1)
8-8: ⚡ Quick winUse
@srcalias for source import in backend app tests.
../../types/providershould use the configured source alias to avoid brittle relative paths.Suggested change
-import type { ProviderWithClusterState } from "../../types/provider"; +import type { ProviderWithClusterState } from "@src/types/provider";As per coding guidelines:
apps/**/*.ts: Use path aliases@src/*for source files and@test/*for test files in backend applications.🤖 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 `@apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts` at line 8, The import uses a brittle relative path for ProviderWithClusterState; replace the relative import of "../../types/provider" with the configured source alias import (e.g., import type { ProviderWithClusterState } from "@src/types/provider") so tests use the backend `@src` path alias; update any other similar relative imports in bid-screening.service.spec.ts to use `@src/`* aliases.apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts (1)
9-9: ⚡ Quick winSwitch this type import to
@srcalias.Use the backend alias for source imports instead of a relative path.
Suggested change
-import type { ProviderWithClusterState } from "../../types/provider"; +import type { ProviderWithClusterState } from "@src/types/provider";As per coding guidelines:
apps/**/*.ts: Use path aliases@src/*for source files and@test/*for test files in backend applications.🤖 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 `@apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts` at line 9, Replace the relative type import for ProviderWithClusterState with the backend path alias: change the import statement that currently references "../../types/provider" to use "@src/types/provider" so the file imports ProviderWithClusterState via the alias (update the import at the top of bid-screening.service.ts where ProviderWithClusterState is imported).
🤖 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 `@apps/provider-inventory/src/config/env.config.ts`:
- Line 13: The Zod schema requires REST_API_NODE_URL (symbol: REST_API_NODE_URL
in envSchema) but the default .env is missing it, causing envSchema.parse() to
fail at startup; add REST_API_NODE_URL to the default .env with an appropriate
URL value or provide a sensible fallback/default in env.config.ts (e.g., make
REST_API_NODE_URL optional or z.string().url().default(...) or read from
process.env with a fallback) so that apps/provider-inventory can boot in
non-test environments; ensure any change references REST_API_NODE_URL in
envSchema and keeps validation via envSchema.parse() consistent with deployment
expectations.
In
`@apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts`:
- Around line 43-51: The parseQuantity path currently converts strings to Number
then to BigInt (in parseQuantity using BINARY_SI, DECIMAL_SI and toBigInt),
which loses precision for large/suffixed values; change it to bigint-safe math:
parse the mantissa as integer and fractional parts (or reject fractions),
convert both parts to BigInt, multiply by the appropriate scale factor from
BINARY_SI/DECIMAL_SI (also expressed as BigInt), adjust for any fraction by
dividing/multiplying to preserve precision, and return the final BigInt directly
via toBigInt only from BigInt values; alternatively replace the function with a
well-tested bigint-safe quantity parser library and wire it in place of
parseQuantity/BINARY_SI/DECIMAL_SI conversions.
In
`@apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.ts`:
- Around line 156-158: The hydratePair function currently calls
BigInt(pair.allocatable) and BigInt(pair.allocated) which will throw if inputs
are null/undefined/non-numeric; create a small sanitizer helper (e.g.,
sanitizeToBigInt) and use it in hydratePair to defensively convert values: if
value is null/undefined/empty -> return 0n, if it matches an integer digit
string -> BigInt(string), else if Number(value) is finite ->
BigInt(Math.trunc(Number(value))), otherwise return 0n (or another safe default)
so ResourcePair always receives valid BigInt values; update hydratePair to call
sanitizeToBigInt(pair.allocatable) and sanitizeToBigInt(pair.allocated).
- Around line 134-142: hydrateClusterState currently casts raw to RawCluster
without validation and will throw if raw is not an object or if
cluster.nodes/cluster.storage are malformed; update hydrateClusterState to
defensively validate input: ensure raw is an object (fallback to {}), coerce
cluster.nodes to an array (e.g. treat non-arrays as empty) before mapping with
hydrateNode, ensure cluster.storage is an object (fallback to {}) before
Object.entries and when reading pool.quantity call hydratePair only if present,
and optionally wrap the parsing in a try/catch to throw or log a clearer error;
reference functions/types: hydrateClusterState, RawCluster, hydrateNode,
hydratePair.
---
Nitpick comments:
In `@apps/provider-inventory/src/providers/chain-sdk.provider.ts`:
- Line 6: Replace the relative import for the APP_CONFIG provider with the
backend path alias: find the import statement importing APP_CONFIG (in
chain-sdk.provider.ts) and change it from a relative path
("./app-config.provider") to the `@src/`* alias equivalent so the file uses the
repository-wide backend alias (e.g., import APP_CONFIG from the aliased module)
to comply with the apps/** import rule.
In
`@apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts`:
- Line 8: The import uses a brittle relative path for ProviderWithClusterState;
replace the relative import of "../../types/provider" with the configured source
alias import (e.g., import type { ProviderWithClusterState } from
"@src/types/provider") so tests use the backend `@src` path alias; update any
other similar relative imports in bid-screening.service.spec.ts to use `@src/`*
aliases.
In `@apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts`:
- Line 9: Replace the relative type import for ProviderWithClusterState with the
backend path alias: change the import statement that currently references
"../../types/provider" to use "@src/types/provider" so the file imports
ProviderWithClusterState via the alias (update the import at the top of
bid-screening.service.ts where ProviderWithClusterState is imported).
In `@apps/provider-inventory/src/types/inventory.ts`:
- Line 1: The import line using a relative path ("import type { ClusterState }
from \"./inventory.types\";") should use the backend source path alias instead;
update the import to reference the aliased module (e.g., import type {
ClusterState } from "@src/types/inventory.types" or the correct `@src/`* path that
maps to this file) so it uses the `@src/`* alias convention, and verify the
referenced symbol ClusterState and the module name match the aliased location.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6af6d7d-766b-4612-857e-d8df8498673e
📒 Files selected for processing (32)
apps/provider-inventory/.eslintrc.jsapps/provider-inventory/env/.env.functional.testapps/provider-inventory/env/.env.sandboxapps/provider-inventory/src/config/env.config.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.tsapps/provider-inventory/src/lib/project-row/project-row.spec.tsapps/provider-inventory/src/lib/project-row/project-row.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.tsapps/provider-inventory/src/lib/resource-pair/resource-pair.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.tsapps/provider-inventory/src/providers/chain-sdk.provider.tsapps/provider-inventory/src/providers/index.tsapps/provider-inventory/src/providers/provider-stream.provider.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.tsapps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.tsapps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.tsapps/provider-inventory/src/types/inventory.tsapps/provider-inventory/src/types/provider.tsapps/provider-inventory/src/types/stream-status.tsapps/provider-inventory/test/functional/discovery-pipeline.spec.tspackages/docker/.env.sandbox.docker-compose-dev
💤 Files with no reviewable changes (5)
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.ts
- apps/provider-inventory/src/types/stream-status.ts
- apps/provider-inventory/src/providers/provider-stream.provider.ts
- apps/provider-inventory/src/providers/index.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.ts
stalniy
force-pushed
the
refactor/reduce-amount-of-remappings
branch
5 times, most recently
from
c88d9b4 to
bce285f
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.ts (1)
20-22: ⚡ Quick winAvoid eager deep-cloning the whole cluster in
match.Line 21 currently deep-clones every node and storage pool, but
#tryAdjustalready clones node/storage for tentative allocation. This doubles cloning work in a hot path and can offset the intended speedup.⚡ Suggested lightweight clone
function cloneCluster(cluster: ClusterState): ClusterState { return { - nodes: cluster.nodes.map(copyNode), - storage: Object.values(cluster.storage).reduce<ClusterState["storage"]>((acc, pool) => { - acc[pool.class] = { class: pool.class, quantity: pool.quantity.clone() }; - return acc; - }, Object.create(null)) + nodes: [...cluster.nodes], + storage: cluster.storage }; }Also applies to: 190-198
🤖 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 `@apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.ts` around lines 20 - 22, The match method currently deep-clones the entire cluster up-front (cloneCluster) before calling `#adjust`, causing duplicate cloning because `#tryAdjust` already performs per-node/per-pool cloning for tentative allocations; instead, remove the eager clone and pass the original ClusterState into match -> `#adjust` and let `#adjust/`#tryAdjust perform lightweight, targeted clones only when modifying a node or storage pool (e.g., where cloneNode/clonePool or per-resource cloning is done). Update the other similar call sites noted (around the block at the 190–198 region) to the same pattern so cloning happens only in the tentative allocation paths.
🤖 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.
Nitpick comments:
In
`@apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.ts`:
- Around line 20-22: The match method currently deep-clones the entire cluster
up-front (cloneCluster) before calling `#adjust`, causing duplicate cloning
because `#tryAdjust` already performs per-node/per-pool cloning for tentative
allocations; instead, remove the eager clone and pass the original ClusterState
into match -> `#adjust` and let `#adjust/`#tryAdjust perform lightweight, targeted
clones only when modifying a node or storage pool (e.g., where
cloneNode/clonePool or per-resource cloning is done). Update the other similar
call sites noted (around the block at the 190–198 region) to the same pattern so
cloning happens only in the tentative allocation paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 280d478f-63cc-4923-a08a-d58a60383624
📒 Files selected for processing (29)
apps/provider-inventory/.eslintrc.jsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.tsapps/provider-inventory/src/lib/project-row/project-row.spec.tsapps/provider-inventory/src/lib/project-row/project-row.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.tsapps/provider-inventory/src/lib/resource-pair/resource-pair.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.tsapps/provider-inventory/src/providers/index.tsapps/provider-inventory/src/providers/provider-stream.provider.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.tsapps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.tsapps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.tsapps/provider-inventory/src/types/inventory.tsapps/provider-inventory/src/types/provider.tsapps/provider-inventory/src/types/stream-status.tsapps/provider-inventory/test/functional/discovery-pipeline.spec.tspackages/docker/.env.sandbox.docker-compose-dev
💤 Files with no reviewable changes (5)
- apps/provider-inventory/src/providers/index.ts
- apps/provider-inventory/src/types/stream-status.ts
- apps/provider-inventory/src/providers/provider-stream.provider.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- apps/provider-inventory/.eslintrc.js
- apps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.ts
- apps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.ts
- apps/provider-inventory/src/types/inventory.ts
- apps/provider-inventory/src/types/provider.ts
- apps/provider-inventory/src/lib/project-row/project-row.spec.ts
- apps/provider-inventory/test/functional/discovery-pipeline.spec.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.ts
- apps/provider-inventory/src/lib/project-row/project-row.ts
- apps/provider-inventory/src/lib/resource-pair/resource-pair.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.ts
- apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts (1)
9-9: ⚡ Quick winUse backend path alias import for provider type.
Line 9 introduces a relative backend import; switch it to
@src/*alias for consistency with app rules.Suggested change
-import type { ProviderWithClusterState } from "../../types/provider"; +import type { ProviderWithClusterState } from "@src/types/provider";As per coding guidelines, "apps/**/*.ts: Use path aliases
@src/*for source files and@test/*for test files in backend applications".🤖 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 `@apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts` at line 9, Replace the relative import of the ProviderWithClusterState type in bid-screening.service.ts with the backend path-alias import; locate the import statement that references "../../types/provider" and change it to use the `@src` alias (e.g., import type { ProviderWithClusterState } from "@src/..." ) so the service (bid-screening.service.ts) follows the apps/** path-alias convention.
🤖 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
`@apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts`:
- Around line 105-108: The loop over storage currently assigns result[cls] = {
class: cls, quantity: pairFromSdk(pool.quantity) } which overwrites earlier
pools with the same cls; instead, detect existing entry in result for that cls
and accumulate quantities (e.g., existing.quantity = addPair(existing.quantity,
pairFromSdk(pool.quantity))) so totals per class are summed; if no helper
exists, implement a small routine to add the numeric fields of the pair returned
by pairFromSdk and use it when updating result[cls] in the loop that iterates
over storage and pool.
In
`@apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.ts`:
- Line 48: ResourcePair instances are serializing to {} when written to the
JSONB inventory column because the class uses private fields (`#allocatable`,
`#allocated`) and lacks a toJSON; add a toJSON() method on the ResourcePair class
that returns { allocatable: this.#allocatable, allocated: this.#allocated } so
that writing row.cluster to the JSONB column preserves values and hydratePair()
will correctly read allocatable/allocated instead of getting undefined/0n.
---
Nitpick comments:
In `@apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts`:
- Line 9: Replace the relative import of the ProviderWithClusterState type in
bid-screening.service.ts with the backend path-alias import; locate the import
statement that references "../../types/provider" and change it to use the `@src`
alias (e.g., import type { ProviderWithClusterState } from "@src/..." ) so the
service (bid-screening.service.ts) follows the apps/** path-alias convention.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 88a23c99-e300-4810-b52b-a48e0eaa1fa6
📒 Files selected for processing (29)
apps/provider-inventory/.eslintrc.jsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.tsapps/provider-inventory/src/lib/project-row/project-row.spec.tsapps/provider-inventory/src/lib/project-row/project-row.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.tsapps/provider-inventory/src/lib/resource-pair/resource-pair.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.tsapps/provider-inventory/src/providers/index.tsapps/provider-inventory/src/providers/provider-stream.provider.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.tsapps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.tsapps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.tsapps/provider-inventory/src/types/inventory.tsapps/provider-inventory/src/types/provider.tsapps/provider-inventory/src/types/stream-status.tsapps/provider-inventory/test/functional/discovery-pipeline.spec.tspackages/docker/.env.sandbox.docker-compose-dev
💤 Files with no reviewable changes (7)
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.ts
- apps/provider-inventory/src/providers/index.ts
- apps/provider-inventory/src/providers/provider-stream.provider.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.ts
- apps/provider-inventory/src/types/stream-status.ts
✅ Files skipped from review due to trivial changes (1)
- apps/provider-inventory/.eslintrc.js
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/provider-inventory/src/types/provider.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.ts
- apps/provider-inventory/src/types/inventory.ts
- apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.ts
- apps/provider-inventory/src/lib/resource-pair/resource-pair.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.ts
- apps/provider-inventory/test/functional/discovery-pipeline.spec.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.ts
- apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts
stalniy
force-pushed
the
refactor/reduce-amount-of-remappings
branch
2 times, most recently
from
1a9d641 to
1d6a917
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts (1)
103-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMerge storage pools by class instead of overwriting them.
If the SDK emits multiple pools for the same class, the last assignment wins and earlier capacity disappears, which undercounts persistent storage during matching.
Proposed change
function mapClusterStorage(storage: SdkStorage[]): ClusterState["storage"] { const result: ClusterState["storage"] = Object.create(null); for (const pool of storage) { const cls = pool.info?.class ?? ""; - result[cls] = { class: cls, quantity: pairFromSdk(pool.quantity) }; + const quantity = pairFromSdk(pool.quantity); + const current = result[cls]; + + result[cls] = current + ? { + class: cls, + quantity: new ResourcePair( + current.quantity.allocatable + quantity.allocatable, + current.quantity.allocated + quantity.allocated + ) + } + : { class: cls, quantity }; } return result; }🤖 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 `@apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts` around lines 103 - 108, mapClusterStorage currently overwrites storage entries by class so multiple SDK pools with the same pool.info.class lose earlier capacity; update the mapClusterStorage function to merge pools by class instead of replacing them: when iterating pools, compute pair = pairFromSdk(pool.quantity) and if result[cls] already exists, add/accumulate the numeric fields from the existing result[cls].quantity and the new pair (e.g., sum bytes/count or whatever numeric properties pairFromSdk returns), otherwise create a new entry; reference symbols: function mapClusterStorage, parameter storage, loop variable pool, cls, result, and pairFromSdk.
🧹 Nitpick comments (2)
apps/provider-inventory/src/types/inventory.ts (1)
1-1: ⚡ Quick winUse the
@srcalias for this source import.This new relative import breaks the backend import-path convention for app code.
Proposed change
-import type { ClusterState } from "./inventory.types"; +import type { ClusterState } from "@src/types/inventory.types";As per coding guidelines,
apps/**/*.ts: Use path aliases@src/*for source files and@test/*for test files in backend applications.🤖 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 `@apps/provider-inventory/src/types/inventory.ts` at line 1, The import in inventory.ts uses a relative path ("./inventory.types"); replace it with the project path-alias import using the backend source alias (e.g., import type { ClusterState } from "@src/types/inventory.types") so it follows the apps/** import convention and resolves via the configured `@src` alias; update any other similar relative imports in this file to use `@src` as needed.apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts (1)
4-4: ⚡ Quick winUse the
@srcalias in this spec import.This changed relative import breaks the backend alias convention for app code.
Proposed change
-import type { ClusterState, CpuInfo, GpuInfo, NodeState, RequestedResourceUnit } from "../../types/inventory.types"; +import type { ClusterState, CpuInfo, GpuInfo, NodeState, RequestedResourceUnit } from "@src/types/inventory.types";As per coding guidelines,
apps/**/*.ts: Use path aliases@src/*for source files and@test/*for test files in backend applications.🤖 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 `@apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts` at line 4, The spec currently imports types via a relative path; update the import in cluster-inventory-matcher.service.spec.ts to use the backend source alias instead of the relative path by replacing the import that brings in ClusterState, CpuInfo, GpuInfo, NodeState, and RequestedResourceUnit from "../../types/inventory.types" with the equivalent `@src` path (e.g. import type { ClusterState, CpuInfo, GpuInfo, NodeState, RequestedResourceUnit } from "@src/.../types/inventory.types") so it follows the apps/** alias convention.
🤖 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
`@apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts`:
- Around line 24-31: The three regexes that feed parseMantissa (the binary,
exponent, and decimal patterns) don't accept a leading '+' so strings like
"+1Gi" fall through; update the binary (/^(-?\d...)/), exponent
(/^(-?\d...)[eE].../) and decimal (/^(-?\d...)([numkMGTPE]?)$/) patterns to
accept an optional leading plus (e.g., change the sign group to allow '+' as
well) so they correctly capture the mantissa and pass it to scaleByPow2 and
scaleByPow10; ensure you keep existing handling of '-' and that the same
modified patterns are used where parseMantissa is invoked.
---
Duplicate comments:
In
`@apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.ts`:
- Around line 103-108: mapClusterStorage currently overwrites storage entries by
class so multiple SDK pools with the same pool.info.class lose earlier capacity;
update the mapClusterStorage function to merge pools by class instead of
replacing them: when iterating pools, compute pair = pairFromSdk(pool.quantity)
and if result[cls] already exists, add/accumulate the numeric fields from the
existing result[cls].quantity and the new pair (e.g., sum bytes/count or
whatever numeric properties pairFromSdk returns), otherwise create a new entry;
reference symbols: function mapClusterStorage, parameter storage, loop variable
pool, cls, result, and pairFromSdk.
---
Nitpick comments:
In
`@apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts`:
- Line 4: The spec currently imports types via a relative path; update the
import in cluster-inventory-matcher.service.spec.ts to use the backend source
alias instead of the relative path by replacing the import that brings in
ClusterState, CpuInfo, GpuInfo, NodeState, and RequestedResourceUnit from
"../../types/inventory.types" with the equivalent `@src` path (e.g. import type {
ClusterState, CpuInfo, GpuInfo, NodeState, RequestedResourceUnit } from
"@src/.../types/inventory.types") so it follows the apps/** alias convention.
In `@apps/provider-inventory/src/types/inventory.ts`:
- Line 1: The import in inventory.ts uses a relative path ("./inventory.types");
replace it with the project path-alias import using the backend source alias
(e.g., import type { ClusterState } from "@src/types/inventory.types") so it
follows the apps/** import convention and resolves via the configured `@src`
alias; update any other similar relative imports in this file to use `@src` as
needed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c026d006-a949-4a0a-b551-612092abefce
📒 Files selected for processing (31)
apps/provider-inventory/.eslintrc.jsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.tsapps/provider-inventory/src/lib/jsonb-bigint/jsonb-bigint.column.tsapps/provider-inventory/src/lib/project-row/project-row.spec.tsapps/provider-inventory/src/lib/project-row/project-row.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.tsapps/provider-inventory/src/lib/resource-pair/resource-pair.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.tsapps/provider-inventory/src/model-schemas/provider-inventory/provider-inventory.schema.tsapps/provider-inventory/src/providers/index.tsapps/provider-inventory/src/providers/provider-stream.provider.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.tsapps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.tsapps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.tsapps/provider-inventory/src/types/inventory.tsapps/provider-inventory/src/types/inventory.types.tsapps/provider-inventory/src/types/provider.tsapps/provider-inventory/src/types/stream-status.tsapps/provider-inventory/test/functional/discovery-pipeline.spec.ts
💤 Files with no reviewable changes (8)
- apps/provider-inventory/src/types/inventory.types.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.ts
- apps/provider-inventory/src/providers/index.ts
- apps/provider-inventory/src/types/stream-status.ts
- apps/provider-inventory/src/providers/provider-stream.provider.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.ts
✅ Files skipped from review due to trivial changes (1)
- apps/provider-inventory/src/lib/jsonb-bigint/jsonb-bigint.column.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/provider-inventory/.eslintrc.js
- apps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.ts
- apps/provider-inventory/src/types/provider.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts
- apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.ts
- apps/provider-inventory/src/lib/project-row/project-row.spec.ts
- apps/provider-inventory/src/lib/project-row/project-row.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.ts
- apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.ts
stalniy
force-pushed
the
refactor/reduce-amount-of-remappings
branch
from
1d6a917 to
2142d55
Compare
stalniy
force-pushed
the
refactor/reduce-amount-of-remappings
branch
from
2142d55 to
8233b50
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.ts (1)
10-23: 💤 Low valueConsider wrapping
new URL()for resilience.If
provider.hostUriis malformed,new URL()throws synchronously. Depending on how upstream handles this, an unhandled exception here could terminate the stream lifecycle unexpectedly. The SDK stream errors will propagate to callers, which may be intentional, but the URL construction failure is a distinct class of error worth distinguishing.Optional: wrap with explicit error type
async *openStatusStream(provider: ChainProvider, signal: AbortSignal): AsyncIterable<ClusterState> { - const url = new URL(provider.hostUri); + let url: URL; + try { + url = new URL(provider.hostUri); + } catch { + throw new Error(`Invalid provider hostUri: ${provider.hostUri}`); + } url.port = "8444";🤖 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 `@apps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.ts` around lines 10 - 23, The synchronous new URL(provider.hostUri) call inside openStatusStream can throw on malformed URIs; wrap the URL construction in a try/catch, handle or rethrow a descriptive error (e.g., throw a new TypeError or custom Error mentioning provider.hostUri) so the failure is clearly distinguished from SDK stream errors; after successful URL creation continue to set url.port and call createProviderSDK and sdk.akash.provider.v1.streamStatus as before.
🤖 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.
Nitpick comments:
In
`@apps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.ts`:
- Around line 10-23: The synchronous new URL(provider.hostUri) call inside
openStatusStream can throw on malformed URIs; wrap the URL construction in a
try/catch, handle or rethrow a descriptive error (e.g., throw a new TypeError or
custom Error mentioning provider.hostUri) so the failure is clearly
distinguished from SDK stream errors; after successful URL creation continue to
set url.port and call createProviderSDK and sdk.akash.provider.v1.streamStatus
as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7738a844-6509-470b-a564-e07ecb533a6e
📒 Files selected for processing (31)
apps/provider-inventory/.eslintrc.jsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.tsapps/provider-inventory/src/lib/compute-rollups/compute-rollups.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.tsapps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.tsapps/provider-inventory/src/lib/jsonb-bigint/jsonb-bigint.column.tsapps/provider-inventory/src/lib/project-row/project-row.spec.tsapps/provider-inventory/src/lib/project-row/project-row.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.tsapps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.tsapps/provider-inventory/src/lib/resource-pair/resource-pair.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.tsapps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.tsapps/provider-inventory/src/model-schemas/provider-inventory/provider-inventory.schema.tsapps/provider-inventory/src/providers/index.tsapps/provider-inventory/src/providers/provider-stream.provider.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.tsapps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.tsapps/provider-inventory/src/services/bid-screening/bid-screening.service.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.tsapps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.tsapps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.tsapps/provider-inventory/src/services/provider-stream-factory/provider-stream-factory.sevice.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.tsapps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.tsapps/provider-inventory/src/types/inventory.tsapps/provider-inventory/src/types/inventory.types.tsapps/provider-inventory/src/types/provider.tsapps/provider-inventory/src/types/stream-status.tsapps/provider-inventory/test/functional/discovery-pipeline.spec.ts
💤 Files with no reviewable changes (8)
- apps/provider-inventory/src/providers/index.ts
- apps/provider-inventory/src/types/inventory.types.ts
- apps/provider-inventory/src/providers/provider-stream.provider.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.spec.ts
- apps/provider-inventory/src/types/stream-status.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.ts
- apps/provider-inventory/src/lib/inventory-mapper/inventory-mapper.spec.ts
- apps/provider-inventory/src/lib/compute-rollups/compute-rollups.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- apps/provider-inventory/src/services/discovery-scheduler/discovery-scheduler.service.spec.ts
- apps/provider-inventory/src/lib/jsonb-bigint/jsonb-bigint.column.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.ts
- apps/provider-inventory/src/types/provider.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.ts
- apps/provider-inventory/.eslintrc.js
- apps/provider-inventory/src/lib/stream-status-mapper/stream-status-mapper.spec.ts
- apps/provider-inventory/test/functional/discovery-pipeline.spec.ts
- apps/provider-inventory/src/model-schemas/provider-inventory/provider-inventory.schema.ts
- apps/provider-inventory/src/types/inventory.ts
- apps/provider-inventory/src/lib/resource-pair/resource-pair.ts
- apps/provider-inventory/src/lib/project-row/project-row.ts
- apps/provider-inventory/src/services/stream-lifecycle-manager/stream-lifecycle-manager.service.spec.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.spec.ts
- apps/provider-inventory/src/services/bid-screening/bid-screening.service.spec.ts
- apps/provider-inventory/src/lib/project-row/project-row.spec.ts
- apps/provider-inventory/src/repositories/provider-inventory/provider-inventory.repository.ts
- apps/provider-inventory/src/lib/projected-row-equals/projected-row-equals.spec.ts
- apps/provider-inventory/src/services/cluster-inventory-matcher/cluster-inventory-matcher.service.spec.ts
ygrishajev
approved these changes
May 13, 2026
This was referenced May 15, 2026
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
2 participants
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.
Why
ref CON-308
What
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Tests