feat(#3529): move ThresholdConfig from MetricProvider to individual Metrics#3560
feat(#3529): move ThresholdConfig from MetricProvider to individual Metrics#3560fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
…etrics Refactor the scorecard MetricProvider interface to move threshold configuration from provider-level methods to individual Metric objects. Core interface changes: - Add `threshold: ThresholdConfig` field to the `Metric` type - Remove `getMetricType()`, `getMetric()`, `getMetricThresholds()`, `calculateMetric()`, and `getMetricIds()` from MetricProvider - Make `getMetrics()` and `calculateMetrics()` required on MetricProvider - Consolidate single/batch provider code paths into batch-only Provider migrations: - All providers (dependabot, filecheck, github, jira, openssf, sonarqube) updated to the new interface with thresholds on metric objects - ThresholdResolver API: `resolveProviderThresholds(provider)` → `resolveMetricThresholds(metric, providerId)` - ThresholdResolver API: `resolveEntityThresholds(entity, provider)` → `resolveEntityThresholds(entity, metric, providerId)` - mergeEntityAndProviderThresholds signature updated similarly - PullMetricsByProviderTask consolidated to always use calculateMetrics() - MetricProvidersRegistry uses provider.getMetrics() for all operations All tests updated. API reports regenerated. Note: DatabaseMetricValues tests could not run (missing better-sqlite3 native binding in sandbox). All other 91 test suites (976 tests) passed. Closes #3529 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fullsend-ai-coder
Bot
requested review from
a team,
Eswaraiahsapram,
christoph-jerolimov and
its-mitesh-kumar
as code owners
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3560 +/- ##
==========================================
- Coverage 53.82% 53.81% -0.01%
==========================================
Files 2277 2277
Lines 87047 87014 -33
Branches 24387 24375 -12
==========================================
- Hits 46850 46826 -24
+ Misses 38660 38651 -9
Partials 1537 1537
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
|
🤖 Finished Review · ✅ Success · Started 12:49 AM UTC · Completed 1:03 AM UTC |
ReviewFindingsCritical
Medium
Low
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| metric: Metric, | ||
| providerId: string, | ||
| ): ThresholdConfig { | ||
| return mergeEntityAndProviderThresholds( |
There was a problem hiding this comment.
[medium] logic-error
In setConfiguredThresholds, configured thresholds are stored per providerId (not per metricId) and validated using metrics[0].type. Pre-existing limitation preserved by the PR but now more visible.
| const providerDatasource = metricProvider.getProviderDatasourceId(); | ||
| const metricType = metricProvider.getMetricType(); | ||
| const providerId = metricProvider.getProviderId(); | ||
|
|
There was a problem hiding this comment.
[low] edge-case
If provider.getMetrics() returns empty array, provider is silently ignored.
| return provider.getMetric(); | ||
| throw new NotFoundError( | ||
| `Metric '${metricId}' not found in provider '${provider.getProviderId()}'`, | ||
| ); |
There was a problem hiding this comment.
[low] error-handling-idiom
calculateMetric throws plain Error while getMetric uses NotFoundError for same condition.
|
/fs-fix |
|
🤖 Finished Fix · ❌ Failure · Started 2:42 AM UTC · Completed 4:16 AM UTC |
1 participant
Refactor the scorecard MetricProvider interface to move threshold configuration from provider-level methods to individual Metric objects.
Core interface changes:
threshold: ThresholdConfigfield to theMetrictypegetMetricType(),getMetric(),getMetricThresholds(),calculateMetric(), andgetMetricIds()from MetricProvidergetMetrics()andcalculateMetrics()required on MetricProviderProvider migrations:
updated to the new interface with thresholds on metric objects
resolveProviderThresholds(provider)→resolveMetricThresholds(metric, providerId)resolveEntityThresholds(entity, provider)→resolveEntityThresholds(entity, metric, providerId)All tests updated. API reports regenerated.
Note: DatabaseMetricValues tests could not run (missing better-sqlite3 native binding in sandbox). All other 91 test suites (976 tests) passed.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Closes #3529
Post-script verification
agent/3529-refactor-metric-provider)b57a430ea0c48b4dff885dc812072d5eccb6e0a2..HEAD)