Conversation
1fdf6fe to
d6cb7f8
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
790591b to
5d968ff
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/build-subgraph-with-defaults.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/get-property/get-subgraph-property.query.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/patch/patch-subgraph.handler.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/core/tests/unit/application/usecase-designer/subgraph/build-subgraph-with-defaults.spec.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
5d968ff to
3c9f276
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
3c9f276 to
9717fc0
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
9717fc0 to
38295a8
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
38295a8 to
41bf049
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/get-property/get-subgraph-property.query.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/patch/patch-subgraph.handler.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
41bf049 to
c7506a4
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/get-property/get-subgraph-property.query.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/patch/patch-subgraph.handler.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
c7506a4 to
c22eeae
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/query-services.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/shared/serialize-elements.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/build-subgraph-with-defaults.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/patch/patch-subgraph.handler.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/subgraph-property-definition/db-subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/queries/typeorm-query-services.ts - Full file content skipped due to token limit
Qualcomm AI Review
| * Wipes all CKV/TKV/tag cal data for a module and resets zero-CKV payloads | ||
| * to their factory defaults. Returns a mutation log. | ||
| */ | ||
| wipeCalData( |
There was a problem hiding this comment.
add 2 methods
wipeAllCkvData
wipeAllTkvData
adding zero ckv data is a business logic, it should be in core layer
| switch (def.elementType) { | ||
| case PARAMETER_ELEMENT_TYPE.ConfigElement: { | ||
| return { | ||
| name: def.name ?? '', |
There was a problem hiding this comment.
Don't assign default values if fields have no value.
| command.elements as unknown as ElementCalData[], | ||
| ); | ||
| if (!serialized.ok) { | ||
| throw new InvalidOperationException(serialized.error); |
There was a problem hiding this comment.
Is this invalid operation or on invalid input?
|
Review Findings
|
c22eeae to
64f8522
Compare
There was a problem hiding this comment.
Files Not Fully Analyzed
- packages/api/src/infrastructure-wrapper/persistence/unit-of-work/typeorm-unit-of-work.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/command-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/orchestration/cqrs/registries/query-handler-registry.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/subgraph-property-definition/subgraph-property-def-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/query-services/vcpm-definition/vcpm-definition-query-service.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/module/module.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/ports/persistence/repositories/subgraph/subgraph.repository.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/dto/subgraph-write-result-types.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/subgraph-property-ids/subgraph-property-ids.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-property/update-subgraph-property.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-scenario/update-subgraph-scenario.command.ts - Full file content skipped due to token limit
- packages/core/src/application/usecase-designer/subgraph/update-vsid/update-subgraph-vsid.command.ts - Full file content skipped due to token limit
- packages/core/src/index.ts - Full file content skipped due to token limit
- packages/infrastructure/persistence/src/persistence-typeorm-sqllite/repositories/subgraph/subgraph.repository.ts - Full file content skipped due to token limit
Qualcomm AI Review
| export class TypeOrmModuleRepository implements ModuleRepository { | ||
| private readonly spfModuleFetcher: SpfModuleOverlayFetcher; | ||
| private readonly nodeFetcher: NodeOverlayFetcher; | ||
| private readonly portFetcher: PortOverlayFetcher; | ||
| private readonly ckvOverlayFetcher: CkvOverlayFetcher; | ||
| private readonly tkvOverlayFetcher: TkvOverlayFetcher; | ||
|
|
||
| constructor( | ||
| private readonly writer: PendingChangeWriter, |
There was a problem hiding this comment.
[FUNCTIONALITY - High] Missing moduleNodeFetcher initialization causes runtime error
The TypeOrmModuleRepository class references this.moduleNodeFetcher in the getModulesBySubgraphId and wipeCalData methods (lines 441-461 and 596-600), but this field is never initialized in the constructor. This will cause a runtime error when these methods are called.
Impact: Any attempt to call getModulesBySubgraphId or wipeCalData will result in a TypeError: "Cannot read property 'loadBaselineNodeIdsForSubgraph' of undefined".
Fixed Code Snippet
export class TypeOrmModuleRepository implements ModuleRepository {
private readonly spfModuleFetcher: SpfModuleOverlayFetcher;
private readonly nodeFetcher: NodeOverlayFetcher;
private readonly portFetcher: PortOverlayFetcher;
private readonly ckvOverlayFetcher: CkvOverlayFetcher;
private readonly tkvOverlayFetcher: TkvOverlayFetcher;
private readonly moduleNodeFetcher: ModuleNodeFetcher; // Add this field
constructor(
private readonly writer: PendingChangeWriter,
private readonly manager: EntityManager,
private readonly uow: UnitOfWork,
) {
const editActionsQs = new EditActionsQueryService(manager);
this.spfModuleFetcher = new SpfModuleOverlayFetcher(manager, editActionsQs);
this.nodeFetcher = new NodeOverlayFetcher(manager, editActionsQs);
this.portFetcher = new PortOverlayFetcher(
manager,
editActionsQs,
new IntentFetcher(manager, editActionsQs),
);
this.ckvOverlayFetcher = new CkvOverlayFetcher(
manager,
editActionsQs,
new CkvParameterPayloadFetcher(manager, editActionsQs),
);
this.tkvOverlayFetcher = new TkvOverlayFetcher(manager, editActionsQs);
this.moduleNodeFetcher = new ModuleNodeFetcher(manager, editActionsQs); // Initialize here
}| }), | ||
| ); | ||
| } | ||
|
|
||
| private async writeZeroCkvResets( | ||
| resets: Array<{payloadSystemId: number; defaultValue: Uint8Array | null}>, | ||
| zeroCkvSystemId: number | undefined, | ||
| moduleSystemId: number, | ||
| sessionId: number, | ||
| groupId: string, | ||
| ): Promise<number[]> { |
There was a problem hiding this comment.
[PERFORMANCE - Medium] Potential N+1 query pattern in module calibration wipe
In the wipeCalData method of TypeOrmModuleRepository (lines 464-508), the writeZeroCkvResets method iterates over resets and calls writeDelta for each one sequentially (lines 735-747). This creates N separate database operations instead of batching them.
Impact: For modules with many parameters, this could result in hundreds of individual database writes, significantly impacting performance during scenario transitions.
Fixed Code Snippet
private async writeZeroCkvResets(
resets: Array<{payloadSystemId: number; defaultValue: Uint8Array | null}>,
zeroCkvSystemId: number | undefined,
moduleSystemId: number,
sessionId: number,
groupId: string,
): Promise<number[]> {
if (!zeroCkvSystemId || resets.length === 0) return [];
// Batch all payload updates into a single operation
await this.writer.writeDeltaBatch(
resets.map(reset => ({
targetTable: ENTITY_NAMES.CkvParameterPayload,
targetSystemId: reset.payloadSystemId,
aggregateId: moduleSystemId,
delta: {payload: reset.defaultValue},
})),
sessionId,
groupId,
this.manager,
);
return [zeroCkvSystemId];
}a39ae9c to
af5bc51
Compare
add requirements for set subgraph property data Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
add llds for set subgraph property data Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
add llds for set subgraph property data Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
add end-to-end support for set subgraph data Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
add lld for review comments Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
fixed review comments Signed-off-by: Robin Gupta <robigupt@qti.qualcomm.com>
af5bc51 to
38babd2
Compare
add requirements for set subgraph property data