What happened
PR #4631 added E2E test coverage for the LICENSE_FILE_EXISTS KPI in the Scorecard plugin. The review agent (first review, Sep 8) flagged a medium-severity finding: mockAllDefaultHomepageAggregationsSuccess in mockHomepageAggregations.ts was not updated to mock the new LICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTE. This finding was raised in all 3 successful review passes (Sep 8, Sep 9, Sep 10) and was independently flagged by Qodo as a "Bug, Correctness" issue. The author addressed 2 low-severity findings but did not address this medium finding. The PR was approved by a human reviewer and merged on Sep 14.
What could go better
The finding is confirmed valid by code analysis. mockAllDefaultHomepageAggregationsSuccess (line 160-192 of mockHomepageAggregations.ts) mocks 5 of 6 KPI aggregation routes but omits LICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTE. The other two analogous functions (mockHomepageAggregationsPermissionDenied and mockAggregationNoDataFound) were correctly updated for the new KPI — only the success mock was missed. The Manage scorecards on Home page test (scorecard.test.ts line 457+) calls this function and then iterates over all 6 entries in AGGREGATED_CARDS_METRIC_IDS including licenseFileExistsKpi, meaning the license card renders without a success mock interceptor. The response data (licenseFileExistsAggregatedResponse) already exists in scorecardResponseUtils.ts but is not imported. Confidence is high — two independent AI review tools identified the same gap, and the code analysis confirms the contract violation.
Proposed change
In workspaces/scorecard/packages/app-legacy/e2e-tests/utils/mockHomepageAggregations.ts: (1) Add licenseFileExistsAggregatedResponse to the import from scorecardResponseUtils (line 22-33). (2) Add a mockApiResponse call for ScorecardRoutes.LICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTE with licenseFileExistsAggregatedResponse inside mockAllDefaultHomepageAggregationsSuccess, before the page.reload() call (around line 191). This is a two-line change that aligns the function with its documented contract and with the other two mock-all functions.
Validation criteria
After the fix: (1) mockAllDefaultHomepageAggregationsSuccess mocks all 6 routes referenced by AGGREGATED_CARDS_METRIC_IDS. (2) The Manage scorecards on Home page E2E test passes with the license card receiving a proper success response. (3) The review agent no longer flags this finding on subsequent PRs touching this area.
Generated by retro agent from #4631
What happened
PR #4631 added E2E test coverage for the LICENSE_FILE_EXISTS KPI in the Scorecard plugin. The review agent (first review, Sep 8) flagged a medium-severity finding:
mockAllDefaultHomepageAggregationsSuccessinmockHomepageAggregations.tswas not updated to mock the newLICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTE. This finding was raised in all 3 successful review passes (Sep 8, Sep 9, Sep 10) and was independently flagged by Qodo as a "Bug, Correctness" issue. The author addressed 2 low-severity findings but did not address this medium finding. The PR was approved by a human reviewer and merged on Sep 14.What could go better
The finding is confirmed valid by code analysis.
mockAllDefaultHomepageAggregationsSuccess(line 160-192 ofmockHomepageAggregations.ts) mocks 5 of 6 KPI aggregation routes but omitsLICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTE. The other two analogous functions (mockHomepageAggregationsPermissionDeniedandmockAggregationNoDataFound) were correctly updated for the new KPI — only the success mock was missed. TheManage scorecards on Home pagetest (scorecard.test.tsline 457+) calls this function and then iterates over all 6 entries inAGGREGATED_CARDS_METRIC_IDSincludinglicenseFileExistsKpi, meaning the license card renders without a success mock interceptor. The response data (licenseFileExistsAggregatedResponse) already exists inscorecardResponseUtils.tsbut is not imported. Confidence is high — two independent AI review tools identified the same gap, and the code analysis confirms the contract violation.Proposed change
In
workspaces/scorecard/packages/app-legacy/e2e-tests/utils/mockHomepageAggregations.ts: (1) AddlicenseFileExistsAggregatedResponseto the import fromscorecardResponseUtils(line 22-33). (2) Add amockApiResponsecall forScorecardRoutes.LICENSE_FILE_EXISTS_KPI_AGGREGATION_ROUTEwithlicenseFileExistsAggregatedResponseinsidemockAllDefaultHomepageAggregationsSuccess, before thepage.reload()call (around line 191). This is a two-line change that aligns the function with its documented contract and with the other two mock-all functions.Validation criteria
After the fix: (1)
mockAllDefaultHomepageAggregationsSuccessmocks all 6 routes referenced byAGGREGATED_CARDS_METRIC_IDS. (2) TheManage scorecards on Home pageE2E test passes with the license card receiving a proper success response. (3) The review agent no longer flags this finding on subsequent PRs touching this area.Generated by retro agent from #4631