feat: secondary manifest repository signal (CM-1393) - #4570
Conversation
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
npm, nuget, rubygems, and pypi never called removeDeclaredPackageRepo when the manifest no longer resolved, leaving old links dangling. pypi upsertProject also lacked the host gate for secondary signals, letting host='other' repos from free-form fields slip through. Also: remove unused `field` from ResolvedManifestRepo, drop oversized JSDoc blocks, update ADR-0021 to match actual Maven behaviour and remove Alternatives Considered. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
… host gate earlier (CM-1393) Packagist: when primary SCM field is absent but homepage resolves to a secondary repo, packages.repository_url stayed null while package_repos got a link — both representations now reflect the resolved URL. PyPI: host gate for secondary signals was applied after upsertPypiPackage received repo.url, letting host='other' values land in packages.repository_url. Gate now runs before the package upsert. Also removes stale `field` assertions from resolveManifestRepo tests. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
writeRepoLink exited early on null or unparseable URLs without removing previous declared links, and on success never cleared links to other repos. Now calls removeDeclaredPackageRepo in all three paths. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…1393) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…normalize (CM-1393) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…-1393) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…94 (CM-1393) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…CM-1393) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Adds lower-confidence secondary manifest repository signals from fallback package metadata across supported ecosystems.
Changes:
- Adds
primary/secondarysignal persistence and confidence scoring. - Integrates ordered fallback resolution across registry workers.
- Adds migration, tests, reconciliation logic, and ADR documentation.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
services/libs/data-access-layer/src/packages/repos.ts |
Persists repository signals. |
services/libs/data-access-layer/src/packages/repoConfidenceWrites.integration.test.ts |
Updates write and rescore integration coverage. |
services/libs/data-access-layer/src/packages/repoConfidenceScoring.integration.test.ts |
Tests secondary-signal scoring. |
services/libs/data-access-layer/src/packages/repoConfidence.ts |
Adds signal types and SQL scoring support. |
services/libs/data-access-layer/src/packages/repoConfidence.test.ts |
Tests signal defaults and SQL generation. |
services/libs/data-access-layer/src/packages/packages.ts |
Returns homepage data and sets repository URLs. |
services/apps/packages_worker/src/utils/resolveManifestRepo.ts |
Adds ordered manifest fallback resolution. |
services/apps/packages_worker/src/utils/__tests__/resolveManifestRepo.test.ts |
Tests fallback ordering and host gating. |
services/apps/packages_worker/src/scripts/rescorePackageRepos.ts |
Updates the scoring migration reference. |
services/apps/packages_worker/src/rubygems/types.ts |
Models resolved repository signals. |
services/apps/packages_worker/src/rubygems/runRubyGemsCoreLoop.ts |
Persists and reconciles RubyGems links. |
services/apps/packages_worker/src/rubygems/normalize.ts |
Adds homepage and tracker fallbacks. |
services/apps/packages_worker/src/pypi/upsertProject.ts |
Persists PyPI repository signals. |
services/apps/packages_worker/src/pypi/normalize.ts |
Classifies source and fallback URLs. |
services/apps/packages_worker/src/pypi/__tests__/normalize.test.ts |
Tests PyPI URL classification. |
services/apps/packages_worker/src/packagist/upsertPackageInfo.ts |
Adds stored-homepage fallback linking. |
services/apps/packages_worker/src/packagist/__tests__/persistPackageInfo.test.ts |
Tests Packagist fallback persistence. |
services/apps/packages_worker/src/packagist/__tests__/dueSelection.test.ts |
Updates homepage result expectations. |
services/apps/packages_worker/src/nuget/types.ts |
Models resolved NuGet signals. |
services/apps/packages_worker/src/nuget/runNuGetEnrichmentLoop.ts |
Persists and reconciles NuGet links. |
services/apps/packages_worker/src/nuget/normalize.ts |
Adds project URL fallback resolution. |
services/apps/packages_worker/src/npm/upsertPackage.ts |
Persists npm fallback signals. |
services/apps/packages_worker/src/npm/types.ts |
Models npm bug URLs. |
services/apps/packages_worker/src/npm/normalize.ts |
Adds homepage and bug URL fallbacks. |
services/apps/packages_worker/src/maven/runMavenEnrichmentLoop.ts |
Adds POM homepage fallback handling. |
services/apps/packages_worker/src/deps-dev/workflows/ingestRepos.ts |
Marks deps.dev links as primary. |
services/apps/packages_worker/src/cargo/types.ts |
Adds Cargo fallback metrics. |
services/apps/packages_worker/src/cargo/normalizeRepos.ts |
Builds Cargo repository choices. |
services/apps/packages_worker/src/cargo/enrich.ts |
Persists Cargo choices and signals. |
docs/adr/README.md |
Indexes ADR-0021. |
docs/adr/0021-secondary-manifest-repository-signal.md |
Documents the fallback signal policy. |
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql |
Adds signal schema and scoring logic. |
Suppressed comments (1)
docs/adr/0021-secondary-manifest-repository-signal.md:107
- These counters are not implemented: the shared resolver discards
field, writers emit no primary/fallback/no-signal metrics, and only Cargo reports a singlehomepageFallbackstotal. Either add the promised per-ecosystem observability or remove this claim from the accepted ADR.
- Per-run counters (`primary_field_hit`, `fallback_hit_by_field`, `no_signal`)
make the coverage uplift measurable against the pre-merge baseline.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…(CM-1393) Add getPackageHomepage helper and secondary-signal homepage fallback for packagist; clear repository_url when maven/nuget/rubygems find no valid repo instead of leaving stale COALESCE state; refactor pypi to an ordered repositoryCandidates list resolved via resolveManifestRepo; fix a packagist double-write and a cargo unconditional link prune. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:13
package_reposis replicated through Sequin, but the explicit Tinybird schema atservices/libs/tinybird/datasources/packageRepos.datasource:14-22does not declaresignal. The new evidence dimension therefore will not be available in Tinybird for Insights or auditing. Add the column to that datasource, including aprimarydefault/backfill strategy for existing rows.
docs/adr/0021-secondary-manifest-repository-signal.md:107- These counters are not implemented: repository search finds these names only in this ADR, and the shared resolver returns no winning field. Cargo logs
homepageFallbacks, but the other writers emit none of the listed per-field counters. Either implement the stated observability or describe the current instrumentation accurately.
docs/adr/0021-secondary-manifest-repository-signal.md:70
- The preceding text counts seven writer copies, but this sentence calls them nine implementations. Use the same count so the alternative is internally consistent.
- **Why not**: the whole point is one rule; nine implementations of "which
field won" is the defect, not the fix.
docs/adr/0021-secondary-manifest-repository-signal.md:13
- This history is inaccurate: the pre-change PyPI writer already fell back to a repo-looking homepage, and NuGet already fell back to
projectUrl. The change adds explicit secondary classification and broadens other ecosystems; it does not introduce fallback linking for every writer from zero. Narrow this statement so the ADR records the actual prior state.
Every registry writer only created a `package_repos` row when the ecosystem's
canonical repository field parsed — npm `repository`, cargo `repository`,
rubygems `source_code_uri`, NuGet `<repository>`, POM `<scm><url>`. A large
share of packages leave that field empty while publishing the same repo URL in
`homepage`, `bugs.url`, `projectUrl`, `bug_tracker_uri`, or the POM `<url>`, and
services/apps/packages_worker/src/pypi/normalize.ts:251
- Dropping the tracker whenever
sourceUrlexists prevents the promised fall-through when that source URL is malformed. With an invalid Source, no repo-looking homepage, and a valid bug tracker,resolveManifestReponever receives the only usable fallback. Keep the tracker last in the candidate list; a valid source will still win first.
if (trackerUrl && !sourceUrl) repositoryCandidates.push({ field: 'bug_tracker', url: trackerUrl })
services/apps/packages_worker/src/pypi/normalize.ts:231
- This four-line comment narrates the candidate-list implementation and exceeds the repository's two-line limit for allowed comments. Condense it to the ordering invariant the caller must preserve.
// Candidates are ordered by trust, most trusted first — a project can declare a Source
// field AND a Homepage/Bug Tracker that also happen to point at a repo host. Keeping all
// of them (rather than picking one before validation) lets the caller fall through to the
// next candidate when the top pick fails canonicalization (malformed URL, unsupported path).
services/apps/packages_worker/src/pypi/upsertProject.ts:46
- This writes a Homepage or bug-tracker fallback into
declared_repository_urlwhenever no Source candidate exists. That column is the raw dedicated repository declaration, while fallback resolution belongs inrepository_urlandpackage_repos.signal; select only thesourcecandidate here so consumers do not receive a fallback field mislabeled as declared.
const declaredRepositoryUrl = repositoryCandidates[0]?.url ?? null
Reconcile the packagist homepage-fallback repo link after phase 2 persists the fresh p2 homepage, instead of only reading whatever homepage phase 1 already had stored - a new package, or one whose homepage just changed, no longer misses its secondary repo link for a full run. Revert the pypi source-URL heuristic to a plain "git" substring match; the word-boundary version stopped matching "GitHub"/"GitLab" labels since "git" has no boundary before the following letter. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Suppressed comments (16)
Previously missed (4) — in code that hasn't changed since the last review.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:13
- Adding the inline CHECK validates every existing
package_reposrow whileALTER TABLEholds its strong lock. On this large replicated table that can block ingestion for the duration of the scan. Add the constraint asNOT VALID, then validate it separately under the weaker validation lock.
services/apps/packages_worker/src/pypi/upsertProject.ts:46 - When only a homepage or bug tracker is available, this writes that fallback into
packages.declared_repository_url. The fallback is supposed to backfillrepository_url, whiledeclared_repository_urlrepresents the canonical Source/Repository field; select only thesourcecandidate here.
docs/adr/0021-secondary-manifest-repository-signal.md:107 - These counters are not implemented by this change: the resolver never returns or records the winning field, and only Cargo logs a single
homepageFallbackscount. Update this consequence to describe the telemetry that actually exists, or add the promised per-ecosystem counters.
services/libs/data-access-layer/src/packages/repoConfidence.ts:73 - The new same-source/cross-source
signalconflict policy is not exercised by the write integration suite: its stored-row helper does not selectsignal, and no write passessecondary. Add assertions that same-source refreshes replace the signal and weaker cross-source claims retain it, since this is the central persistence invariant introduced here.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:6
- The migration header is a multi-line summary of the change and exceeds the repository's two-line comment maximum (
CLAUDE.md:72-84). Keep only the non-obvious existing-row behavior.
-- Extends the confidence scoring introduced in V1788307200 with the manifest field
-- that produced a declared link. A repo URL read from a fallback field (homepage,
-- bug_tracker) is weaker evidence than one read from the dedicated repository field,
-- so it lands one tier lower.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:72
- This performance explanation exceeds the repository's two-line comment maximum (
CLAUDE.md:72-84). Condense it to the ordering invariant the arithmetic must preserve.
IF p_disabled IS TRUE THEN
-- Scale proportionally so pre-disabled claim ordering is preserved across sources.
-- The offset uses a tighter modulo so max contribution (3*1000+999)*1e-9 ≈ 4e-6
-- stays below the 0.00016 minimum scaled tier gap and cannot invert source ordering.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:92
- This tie-break explanation exceeds the repository's two-line comment maximum (
CLAUDE.md:72-84). Keep the deterministic-collision invariant without restating the implementation.
-- Tie-breaker: reduces same-source collisions to the rare case where two repo IDs for
-- the same package are congruent mod 1,000,000. BEST_REPO_LINK_JOIN uses a secondary
-- ORDER BY repo_id DESC as the canonical deterministic pick when confidence ties.
backend/src/osspckgs/migrations/V1788307300__package_repo_signal.sql:132
- This legacy-provenance explanation exceeds the repository's two-line comment maximum (
CLAUDE.md:72-84). It can preserve the safety invariant in one line.
-- deps_dev rows with NULL provenance were ingested before this column existed;
-- skip them so the backfill does not downgrade SLSA/attestation links to 0.50.
-- They will be rescored correctly once the next ingest populates provenance.
docs/adr/0021-secondary-manifest-repository-signal.md:13
- This context is historically inaccurate: the removed PyPI and NuGet implementations already created limited homepage/projectUrl fallback links. The new behavior distinguishes and broadens fallback links rather than introducing them for every writer, so describe that distinction to keep the ADR consistent with the code and migration trade-off.
Every registry writer only created a `package_repos` row when the ecosystem's
canonical repository field parsed — npm `repository`, cargo `repository`,
rubygems `source_code_uri`, NuGet `<repository>`, POM `<scm><url>`. A large
share of packages leave that field empty while publishing the same repo URL in
`homepage`, `bugs.url`, `projectUrl`, `bug_tracker_uri`, or the POM `<url>`, and
services/apps/packages_worker/src/cargo/enrich.ts:76
- This explanatory block exceeds the repository's two-line comment limit (
CLAUDE.md:72-84). The only non-obvious detail is why nullable direct assignment is required.
-- repo_choice has exactly one row per package_id in enrich_packages (a plain FROM,
-- never filtered), so rn.repository_url is authoritative for this run — direct-assign
-- it (not COALESCE) so a package that lost its repo in this dump has the denormalized
-- column cleared consistently with enrichRepos' unconditional package_repos prune.
services/apps/packages_worker/src/cargo/enrich.ts:209
- This comment exceeds the repository's two-line maximum (
CLAUDE.md:72-84). Condense it to the churn-avoidance invariant; the SQL already shows the source and target predicates.
// Prunes cargo-owned declared links whose target changed since the last run: removals
// (NULL in this dump is authoritative — loadDump stages every crate every run), URL
// rewrites, and junk/unparseable values. An unchanged link is left alone so the upsert
// below's ON CONFLICT ... KEEP_HIGHEST_CONFLICT_UPDATE handles same-repo signal/confidence
// changes (e.g. a primary→secondary downgrade) without a delete+reinsert. Scoped to
services/apps/packages_worker/src/packagist/tests/persistPackageInfo.test.ts:277
- This explanatory test header exceeds the repository's two-line comment limit (
CLAUDE.md:72-84), while thedescribename already states the subject. Remove the redundant block.
// Phase 1 links a homepage-fallback repo from whatever homepage is already stored;
// this reconciles it once phase 2 has persisted a fresh homepage for a package that
// had none yet (a new package, or one whose homepage just changed).
services/apps/packages_worker/src/packagist/activities.ts:214
- This three-line change note exceeds the repository's two-line comment limit (
CLAUDE.md:72-84) and duplicates the reconciliation helper's contract. Keep only the ordering condition that explains this call site.
// Phase 1's homepage-fallback repo link used whatever homepage was already stored —
// for a new package, or one whose homepage just changed, that's stale/absent until
// this p2 write lands it. Reconcile now so the link doesn't wait for the next run.
services/apps/packages_worker/src/packagist/upsertPackageInfo.ts:57
- This comment exceeds the repository's two-line limit (
CLAUDE.md:72-84). Condense it to the phase-ordering invariant that is not apparent from the query itself.
// The version manifests carry the homepage, not this endpoint — peek at the currently
// stored homepage so a package that only declares a homepage still gets a link, without
// a second write once the stats row is updated below.
services/apps/packages_worker/src/packagist/upsertPackageInfo.ts:123
- This five-line change note exceeds the repository's two-line maximum and mostly restates the reconciliation flow (
CLAUDE.md:72-84). Retain only the ordering constraint callers need to know.
// Phase 1 (dynamic endpoint) resolves the homepage-fallback repo from whatever homepage
// is already stored, but the p2 endpoint (phase 2) is what actually carries a new/changed
// homepage — see ingestOnePackagistMetadata. Called after phase 2 persists, so a package
// with no declared repository field still gets linked to its homepage in the same run it's
// first seen, instead of waiting for the next scheduled ingestion.
services/apps/packages_worker/src/pypi/normalize.ts:231
- This explanatory block exceeds the repository's two-line comment limit (
CLAUDE.md:72-84). Keep only the non-obvious ordering invariant; the candidate construction already expresses the rest.
// Candidates are ordered by trust, most trusted first — a project can declare a Source
// field AND a Homepage/Bug Tracker that also happen to point at a repo host. Keeping all
// of them (rather than picking one before validation) lets the caller fall through to the
// next candidate when the top pick fails canonicalization (malformed URL, unsupported path).
| `UPDATE packages SET repository_url = $(url) | ||
| WHERE id = $(packageId)::bigint AND repository_url IS DISTINCT FROM $(url)`, |
| qx, | ||
| candidate.purl, | ||
| phase1.packageId, | ||
| persistResult.homepage, |
| if (homepage && REPO_HOST.test(homepage)) { | ||
| repositoryCandidates.push({ field: 'homepage', url: homepage }) | ||
| } | ||
| if (trackerUrl && !sourceUrl) repositoryCandidates.push({ field: 'bug_tracker', url: trackerUrl }) |
What
Packages only ever got a
package_reposrow when the ecosystem's canonicalrepository field parsed — npm
repository, cargorepository, rubygemssource_code_uri, NuGet<repository>, POM<scm><url>. A large share ofpackages leave that field empty while publishing the same repo URL in
homepage,bugs.url,projectUrl,bug_tracker_uri, or POM<url>, sothose packages ended up with no repo link at all — invisible to criticality,
blast radius, and Insights.
This adds a
signaldimension to the link: the canonical field resolves asprimary, a fallback field assecondary, and a secondary link scores lowerthan a declared primary one instead of ranking equally with it.
Changes
V1788307300__package_repo_signal.sql— addspackage_repos.signal text NOT NULL DEFAULT 'primary'(CHECKprimary|secondary), replacespackage_repo_confidence()with a 10-argversion taking
p_signal, and re-createsrescore_package_repo_confidenceto passsignalthrough. The function isdropped before being re-created: adding a parameter changes the signature, so
CREATE OR REPLACEalone would leave both overloads callable.signal = 'secondary', ondeclaredlinks only.deps.dev provenance already proves the publisher→repo relationship
independently of which manifest field carried the URL, and
manuallinks areoperator-pinned. Declared 0.85 → 0.75, maven declared 0.80 → 0.70.
packages_worker/src/utils/resolveManifestRepo.ts— takes anordered candidate list, returns
{ repo, signal }. First candidate is theecosystem's canonical field (
primary), every later one issecondary. Asecondarycandidate is rejected when canonicalization yieldshost === 'other'(recognized VCS hosts only);primarykeeps its historicalpermissive behaviour so existing self-hosted Gitea/cgit/SVN links are
unaffected. No writer computes a confidence value.
nuget, maven. Cargo is set-based SQL over a dump, so it stages both
declared_repository_urlandhomepageintorepo_normand applies the samefirst-wins-with-host-gate rule via a new
repo_choicetable.setPackageRepositoryUrlin the DAL — when a link resolves from afallback field,
packages.repository_urlis backfilled with the resolved URL.signalalongsidesource/provenance: cross-source keep-highest, same-source always replaces.gate.
ownership_matchis deliberately out of scope here — it lands in CM-1394.Notes for review
repoConfidenceScoring,repoConfidenceWrites) cover the new column, the 10-arg function, the −0.10penalty and the rescore path, but they're
describe.skipIf(!HAVE_DB)and werenot run locally — they need
CROWD_PACKAGES_DB_*set andV1788307300applied. Worth running against a real packages-db before merge.cargo/,nuget/,rubygems/anddeps-dev/have no unit test files atall; their changes here are covered only by the shared
resolveManifestRepotests plus review.