Skip to content

feat(scorecard): Introduce collectors contract#3526

Draft
dzemanov wants to merge 4 commits into
redhat-developer:mainfrom
dzemanov:scorecard-collectors
Draft

feat(scorecard): Introduce collectors contract#3526
dzemanov wants to merge 4 commits into
redhat-developer:mainfrom
dzemanov:scorecard-collectors

Conversation

@dzemanov

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

Introduce collectors extension point to use by metric providers to collect data from different datasources.

Fixes

Fixes https://redhat.atlassian.net/browse/RHIDP-13977

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

dzemanov added 3 commits June 19, 2026 10:04
Assisted-By: Cursor Desktop
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Assisted-By: Cursor Desktop
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
Assisted-By: Cursor Desktop
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
@github-actions

Copy link
Copy Markdown
Contributor

This pull request adds a new top-level directory under workspaces/. Please follow Submitting a Pull Request for a New Workspace in CONTRIBUTING.md.

@rhdh-gh-app

rhdh-gh-app Bot commented Jun 22, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-scorecard-backend workspaces/scorecard/plugins/scorecard-backend minor v2.7.9
@red-hat-developer-hub/backstage-plugin-scorecard-node workspaces/scorecard/plugins/scorecard-node minor v2.7.9

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:30 AM UTC · Completed 9:51 AM UTC
Commit: a408de5 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:52 — The collectorRegistry instance is created but never injected into or made available to metric providers. The ScorecardCollectorsExtensionPoint is a structural supertype of CollectorRegistry (it has getCollector/hasCollector), so a module CAN capture the extension point during init() and pass it to its MetricProvider. However, the documentation does not show this wiring pattern, and there is no explicit service ref, making the feature less discoverable.
    Remediation: Either document how modules should wire the extension point as a CollectorRegistry, or create a dedicated Backstage service ref for cleaner dependency injection.

  • [logic-error] workspaces/scorecard/plugins/scorecard-backend/docs/providers.md:9 — Broken relative link: [collectors.md](./docs/collectors.md) resolves to docs/docs/collectors.md because providers.md is already inside the docs/ directory.
    Remediation: Change [collectors.md](./docs/collectors.md) to [collectors.md](./collectors.md).

  • [api-shape-pattern] workspaces/scorecard/plugins/scorecard-node/src/extensions.ts:44ScorecardCollectorsExtensionPoint exposes getCollector and hasCollector as read-access methods, but ScorecardMetricsExtensionPoint only exposes addMetricProvider (write-only). Extension points in Backstage are typically write-only registration APIs. Mixing read and write on the same extension point breaks this convention.
    Remediation: Consider removing getCollector and hasCollector from the extension point and providing CollectorRegistry access through a separate service ref.

  • [error-handling-idiom] workspaces/scorecard/plugins/scorecard-node/src/api/collectWithContract.ts:57collectWithContract uses InputError for all four validation failures, but only the first case (contract input validation) is genuinely an input error. Collector-input schema mismatches, collector output schema failures, and contract output mismatches are integration/wiring errors, not user input errors.
    Remediation: Reserve InputError for caller-supplied input validation. Use a plain Error or ConflictError for schema mismatches between provider and collector.

Low

  • [edge-case] collectWithContract.ts:80 — Zod's default .safeParse() strips unknown keys. If the contract output schema expects a field the collector returns but the collector's own output schema does not declare, the field is silently lost between the two validation steps.
  • [naming-convention] CollectorRegistry.ts:20 — Backend class CollectorRegistry shares its name with the CollectorRegistry interface from scorecard-node. The existing pattern uses distinct names (MetricProvider interface vs MetricProvidersRegistry class).
  • [test-adequacy] collectWithContract.test.ts — No test covers collector.collect() throwing an exception (errors propagate without collector ID context).
  • [test-adequacy] collectWithContract.test.ts — No test covers Zod stripping behavior when the collector returns extra fields not in its output schema.
  • [error-message-consistency] CollectorRegistry.ts:25 — Minor phrasing inconsistency between error messages.
  • [scope-label-mismatch] PR carries do-not-merge/work-in-progress label yet introduces new public API surface with a minor changeset.
  • [scope-creep] package.json:49 — Adding zod as a runtime dependency broadens the dependency surface for all consumers.
  • [documentation-comment-format] extensions.ts:41ScorecardCollectorsExtensionPoint members lack JSDoc comments; report.api.md flags them as (undocumented).
  • [docs-currency] README.md:25 — Workspace-level documentation index table does not include an entry for the new collectors.md.
  • [docs-currency] drill-down.md:552 — Related Documentation section does not mention the new collectors.md.
Previous run

Review

Findings

Medium

  • [error-handling] workspaces/scorecard/plugins/scorecard-node/src/api/collectWithContract.ts:85 — The third and fourth validation checks (collector output vs collector schema at ~line 85, and collector output vs contract schema at ~line 98) throw InputError, but these represent internal errors (the collector produced bad output or there is a schema mismatch), not client input errors. The first two InputError throws (contract input validation and collector input validation) are appropriate. Only the output validation throws are misclassified. In Backstage, InputError maps to HTTP 400, which would mislead callers into thinking they sent bad data.
    Remediation: Use a non-InputError type (e.g., Error or a dedicated CollectorOutputError) for the two output validation failures so callers can distinguish "bad input I provided" from "collector returned unexpected output."

Low / Info

  • [API-contract] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:49 — The collectorRegistry is created in the register scope but how metric providers obtain it at runtime is not explicitly wired. The ScorecardCollectorsExtensionPoint interface structurally satisfies CollectorRegistry (both have getCollector and hasCollector), so modules can depend on the extension point directly — this is standard Backstage practice. Consider adding a complete end-to-end wiring example showing a module that depends on both scorecardMetricsExtensionPoint and scorecardCollectorsExtensionPoint.

  • [edge-cases] workspaces/scorecard/plugins/scorecard-node/src/api/collectWithContract.ts:67 — All four Zod safeParse error messages use .map(issue => issue.message).join('; ') but omit issue.path, which makes validation errors for nested objects ambiguous (e.g., two fields failing with "Required" gives "Required; Required" with no indication of which fields failed).

  • [sub-agent-failure] Three review dimensions (style-conventions, intent-coherence, docs-currency) could not be evaluated due to model availability issues. These dimensions were not assessed in this review.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 22, 2026
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
@sonarqubecloud

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 10:54 AM UTC · Ended 11:09 AM UTC
Commit: a408de5 · View workflow run →

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.16667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.46%. Comparing base (2465411) to head (039281e).
⚠️ Report is 50 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3526      +/-   ##
==========================================
+ Coverage   52.43%   52.46%   +0.03%     
==========================================
  Files        2251     2255       +4     
  Lines       85725    85805      +80     
  Branches    24111    24117       +6     
==========================================
+ Hits        44948    45017      +69     
- Misses      39164    39175      +11     
  Partials     1613     1613              
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from 2465411
ai-integrations 67.95% <ø> (ø) Carriedforward from 2465411
app-defaults 69.79% <ø> (ø) Carriedforward from 2465411
augment 46.39% <ø> (ø) Carriedforward from 2465411
boost 100.00% <ø> (ø) Carriedforward from 2465411
bulk-import 72.46% <ø> (ø) Carriedforward from 2465411
cost-management 14.10% <ø> (ø) Carriedforward from 2465411
dcm 61.79% <ø> (ø) Carriedforward from 2465411
extensions 61.53% <ø> (ø) Carriedforward from 2465411
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 2465411
global-header 59.71% <ø> (ø) Carriedforward from 2465411
homepage 49.84% <ø> (ø) Carriedforward from 2465411
install-dynamic-plugins 56.23% <ø> (ø) Carriedforward from 2465411
konflux 91.49% <ø> (ø) Carriedforward from 2465411
lightspeed 68.57% <ø> (ø) Carriedforward from 2465411
mcp-integrations 85.46% <ø> (ø) Carriedforward from 2465411
orchestrator 37.75% <ø> (ø) Carriedforward from 2465411
quickstart 63.76% <ø> (ø) Carriedforward from 2465411
sandbox 79.56% <ø> (ø) Carriedforward from 2465411
scorecard 83.89% <79.16%> (+0.05%) ⬆️
theme 61.26% <ø> (ø) Carriedforward from 2465411
translations 6.55% <ø> (ø) Carriedforward from 2465411
x2a 57.25% <ø> (ø) Carriedforward from 2465411

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2465411...039281e. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 22, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:54 AM UTC · Completed 11:09 AM UTC
Commit: a408de5 · View workflow run →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant