Skip to content

feat: secondary manifest repository signal (CM-1393) - #4570

Draft
joanagmaia wants to merge 17 commits into
mainfrom
feat/CM-1393-secondary-manifest-signal
Draft

feat: secondary manifest repository signal (CM-1393)#4570
joanagmaia wants to merge 17 commits into
mainfrom
feat/CM-1393-secondary-manifest-signal

Conversation

@joanagmaia

Copy link
Copy Markdown
Contributor

What

Packages only ever got 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 POM <url>, so
those packages ended up with no repo link at all — invisible to criticality,
blast radius, and Insights.

This adds a signal dimension to the link: the canonical field resolves as
primary, a fallback field as secondary, and a secondary link scores lower
than a declared primary one instead of ranking equally with it.

Changes

  • Migration V1788307300__package_repo_signal.sql — adds
    package_repos.signal text NOT NULL DEFAULT 'primary' (CHECK
    primary|secondary), replaces package_repo_confidence() with a 10-arg
    version taking p_signal, and re-creates
    rescore_package_repo_confidence to pass signal through. The function is
    dropped before being re-created: adding a parameter changes the signature, so
    CREATE OR REPLACE alone would leave both overloads callable.
  • −0.10 penalty for signal = 'secondary', on declared links only.
    deps.dev provenance already proves the publisher→repo relationship
    independently of which manifest field carried the URL, and manual links are
    operator-pinned. Declared 0.85 → 0.75, maven declared 0.80 → 0.70.
  • Shared helper packages_worker/src/utils/resolveManifestRepo.ts — takes an
    ordered candidate list, returns { repo, signal }. First candidate is the
    ecosystem's canonical field (primary), every later one is secondary. A
    secondary candidate is rejected when canonicalization yields
    host === 'other' (recognized VCS hosts only); primary keeps its historical
    permissive behaviour so existing self-hosted Gitea/cgit/SVN links are
    unaffected. No writer computes a confidence value.
  • Wired into every registry writer: npm, pypi, cargo, rubygems, packagist,
    nuget, maven. Cargo is set-based SQL over a dump, so it stages both
    declared_repository_url and homepage into repo_norm and applies the same
    first-wins-with-host-gate rule via a new repo_choice table.
  • setPackageRepositoryUrl in the DAL — when a link resolves from a
    fallback field, packages.repository_url is backfilled with the resolved URL.
  • Keep-highest conflict policy (ADR-0020) carries signal alongside
    source/provenance: cross-source keep-highest, same-source always replaces.
  • ADR-0021 documents the decision, the per-ecosystem chains, and the host
    gate.

ownership_match is deliberately out of scope here — it lands in CM-1394.

Notes for review

  • The DAL confidence integration tests (repoConfidenceScoring,
    repoConfidenceWrites) cover the new column, the 10-arg function, the −0.10
    penalty and the rescore path, but they're describe.skipIf(!HAVE_DB) and were
    not run locally — they need CROWD_PACKAGES_DB_* set and
    V1788307300 applied. Worth running against a real packages-db before merge.
  • cargo/, nuget/, rubygems/ and deps-dev/ have no unit test files at
    all; their changes here are covered only by the shared
    resolveManifestRepo tests plus review.

joanagmaia and others added 15 commits September 7, 2026 14:55
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>
Copilot AI balanced review requested due to automatic review settings September 7, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds lower-confidence secondary manifest repository signals from fallback package metadata across supported ecosystems.

Changes:

  • Adds primary/secondary signal 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 single homepageFallbacks total. 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.

Comment thread services/apps/packages_worker/src/cargo/enrich.ts Outdated
Comment thread services/apps/packages_worker/src/cargo/enrich.ts
Comment thread services/apps/packages_worker/src/maven/runMavenEnrichmentLoop.ts
Comment thread services/apps/packages_worker/src/maven/runMavenEnrichmentLoop.ts
Comment thread services/apps/packages_worker/src/nuget/runNuGetEnrichmentLoop.ts
Comment thread services/apps/packages_worker/src/packagist/upsertPackageInfo.ts Outdated
Comment thread services/apps/packages_worker/src/pypi/upsertProject.ts Outdated
Comment thread services/apps/packages_worker/src/rubygems/runRubyGemsCoreLoop.ts
…(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>
Copilot AI review requested due to automatic review settings September 7, 2026 15:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_repos is replicated through Sequin, but the explicit Tinybird schema at services/libs/tinybird/datasources/packageRepos.datasource:14-22 does not declare signal. The new evidence dimension therefore will not be available in Tinybird for Insights or auditing. Add the column to that datasource, including a primary default/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 sourceUrl exists prevents the promised fall-through when that source URL is malformed. With an invalid Source, no repo-looking homepage, and a valid bug tracker, resolveManifestRepo never 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_url whenever no Source candidate exists. That column is the raw dedicated repository declaration, while fallback resolution belongs in repository_url and package_repos.signal; select only the source candidate here so consumers do not receive a fallback field mislabeled as declared.
  const declaredRepositoryUrl = repositoryCandidates[0]?.url ?? null

Comment thread services/apps/packages_worker/src/packagist/upsertPackageInfo.ts
Comment thread services/apps/packages_worker/src/pypi/normalize.ts Outdated
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>
Copilot AI review requested due to automatic review settings September 7, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_repos row while ALTER TABLE holds its strong lock. On this large replicated table that can block ingestion for the duration of the scan. Add the constraint as NOT 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 backfill repository_url, while declared_repository_url represents the canonical Source/Repository field; select only the source candidate 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 homepageFallbacks count. 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 signal conflict policy is not exercised by the write integration suite: its stored-row helper does not select signal, and no write passes secondary. 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 the describe name 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).

Comment on lines +326 to +327
`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 })
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants