Skip to content

Retain same-named Hugging Face repositories from different resource kinds - #773

Merged
ktwu01 merged 4 commits into
ktwu01:mainfrom
rudycelekli:fix/radar-hub-kind-identity-20261006
Oct 8, 2026
Merged

ktwu01 merged 4 commits into
ktwu01:mainfrom
rudycelekli:fix/radar-hub-kind-identity-20261006

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Same-named Hugging Face datasets, models and Spaces were overwritten during collection, merged by long-title deduplication, and collapsed again by public search. The full discovery path now preserves each resource kind: collection uses kind plus upstream ID, native Hub deduplication uses exact artifact identity, and QueryService uses the primary kind-aware Hub identity for latest observations and stable public result keys. Original upstream IDs and snapshot fields are preserved.

Merged current main (569c581) into the branch, retaining both the Hub identity and Zenodo unknown-creator regression blocks. Added query regressions that execute the real collector, shared deduplication, repeated snapshot writes, public search and CLI for short/long same-named repositories and a distinct-name control. A later dataset observation replaces only that kind while its model and Space siblings keep their keys.

Validation on c16bf8f0abb4bc99aa9dec630088d5921aa03c3e:

  • Before the query repair, both same-name cases returned one result and failed; the distinct-name control passed. All three pass after the repair.
  • All six required AGENTS.md commands pass in a fresh detached checkout with its own Python 3.12 environment, Node 22, Bash 5 and initialized pinned recursive submodule. 1,789 tests pass.
  • Fresh regeneration covers 3,217 benchmark records across five sources.
  • Upstream CI passes on the exact final head; the latest Copilot review recommends approval with no findings.

Codex prepared the follow-up and tests; an independent agent reviewed the final source, tests and merge resolution with no findings.

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
@rudycelekli
rudycelekli requested a review from ktwu01 as a code owner October 6, 2026 09:39
Copilot AI balanced review requested due to automatic review settings October 6, 2026 09:39
@rudycelekli

Copy link
Copy Markdown
Contributor Author

330226 gpt-6.1-sol

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Shared deduplication still merges same-named repositories when their normalized name reaches the title-key threshold.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Preserves same-named Hugging Face repositories across resource kinds during collection.

Changes:

  • Keys collected repositories by resource kind and upstream ID.
  • Adds regression coverage through shared deduplication.
File Description
src/​benchmark_radar/​sources.py Uses kind-aware collector keys.
tests/​test_sources.py Tests same-name dataset, model, and Space handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/benchmark_radar/sources.py
Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 10:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation addresses both collector overwrites and downstream deduplication with focused regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

ktwu01 commented Oct 7, 2026

Copy link
Copy Markdown
Owner

This one is kept, and it is the only one of your open PRs we are keeping. It fixes real silent record loss (same-named Hugging Face dataset/model/Space collapsing into one record), and CI was green. #723 and #774 are already merged.

It cannot merge yet: tests/test_sources.py now conflicts with main, because #774 added its test at the end of the same file. src/benchmark_radar/pipeline.py and sources.py auto-merge. To finish:

  1. Merge current main into the branch (or rebase) and keep both test blocks at the end of tests/test_sources.py.
  2. Run the CI sequence from AGENTS.md and push.
  3. Reply here once pushed and it will be merged with a merge commit.

The maintainer cannot push to your fork, so this needs to come from you.


Generated by Claude Code

@toby-bridges toby-bridges left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

动机

审查提交:cd92d393108b87f8a636e18b279006a82fc50e5c。同一个 Hugging Face owner/name 可以同时属于 dataset、model 和 Space;这些是不同资源,应能被分别发现。本 PR 修复了采集阶段的覆盖和长名称的错误去重,方向符合维护者保留这个真实记录丢失修复的要求。但沿实际搜索入口检查后,用户仍只能找到其中一个资源,完整的发现路径尚未修通。

改动思路

实现沿用现有采集器、RadarItem 和共享 artifact 身份,没有增加新状态或独立去重框架。采集字典从 upstream ID 改为 (kind, upstream ID),随后对原生 Hub 记录使用已有的 kind-aware artifact key,避免相同 owner/name 被通用标题规则合并。这样保留原始 source_id 及历史数据格式,改动规模也合适。

正向路径已经实测:三个 kind 经 fetch_huggingface → deduplicate → write_snapshot 后各保留一条;同日重复写入后仍有三条。问题出在后面的 QueryService 仍把 (source, source_id) 当作唯一身份。只修采集器可以恢复存储覆盖,但无法让 CLI/HTTP 的共同查询层返回全部资源。

具体改动

完整 diff 为三个文件,增加 62 行、删除 3 行:两个生产文件和一个现有测试模块;没有修改工作流、依赖、源数据或生成文件。

关键代码讲解

  • sources.py:fetch_huggingface 使用 (kind, item_id) 保存采集结果,因此搜索词重复仍合并同一 kind,而同名的三个 kind 不再在入库前互相覆盖。原始 ID、URL、时间筛选和总量上限保持原行为。
  • pipeline.py:dedupe_keys 对带 Hub artifact 身份的原生 Hugging Face 记录跳过长标题哈希,继续沿用 exact_artifact_keys;其他来源仍保留既有标题匹配。这覆盖了上一轮机器人指出的长名称问题。
  • tests/test_sources.py:test_huggingface_preserves_same_named_repositories_of_different_kinds 覆盖短/长 owner/name、重复观测、各 kind 的指标最大值和证据链接隔离;它的断言停在 deduplicate 返回值,没有经过持久快照后的公共搜索入口。

对主干的风险

[P2] 将 kind-aware 身份贯通到公共搜索,避免保存三条却只查到一条。 新的 found[(kind, item_id)] 会产出三个拥有相同 source="Hugging Face"、source_id="lab/suite" 的记录。查询层 _radar_candidates 随后以 (source, source_id) 写入字典,并生成同样不含 kind 的结果 key,最后一个 Space 覆盖 dataset 和 model。

独立复现只替换上游 HTTP JSON,采集、去重、快照写入/读取、QueryService 和实际 CLI 都执行生产实现:短名 lab/suite 与长名 lab/a-long-benchmark-suite-name 均为 采集 3 → 去重 3 → 重复写入后快照 3 → API/CLI 搜索 1,返回的只有 Space,CLI 退出码仍为 0。相同三个 kind 使用不同 owner/name 的对照返回 3 条。当前 main 的对照在采集时就只剩 1 条:这是本次修复尚未覆盖的下游缺口,不是声称本 PR 新引入了覆盖逻辑。

最小修复是在查询的 latest-observation 身份及输出 key 中复用已有 kind-aware Hub artifact 身份,保留原始 upstream ID;同时保证同一 kind 跨次观测仍只返回最新一条、不同 kind 各有稳定 key。请在现有查询测试中加入“真实采集结果写入快照后经公共搜索返回三条”的回归,并保留不同 ID 的对照。

语义与 CI 对齐

准确 head 在干净检出、独立 Python 3.12 环境和固定递归子模块下按顺序通过 ruff check .、ruff format --check .、benchmark-radar normalize-catalog、benchmark-radar classify、benchmark-radar build-data-release、pytest -q,1,642 项测试通过。该 head 的 Actions 运行 也成功;这不能覆盖测试尚未涉及的查询身份语义。

与当前 main 569c58100f663d6d1b9ce8498b3a7f8bed423973 的 git merge-tree --write-tree 仍在 tests/test_sources.py 冲突。这是维护者已报告的事项,不重复追加为新发现;解决时须保留双方测试。此处没有声称合并结果已通过 CI。

我的整体评价

采集和共享去重的局部修复有效,复用已有身份机制的设计也合理;没有必要引入新的存储字段或重写上游 ID。持续重复采集能够保留三条快照证据,但用户经公共搜索仍丢失两个结果,因此“可保存”和“可发现”还没有一致。建议完成上述最小查询修复、保留跨次最新观测语义并解决既有冲突后再验收。主干整合后的完整 CI 与修复后的搜索回归仍待验证。

English verdict: REQUEST_CHANGES — head cd92d39. Collection, deduplication and repeated snapshot writes preserve three same-named Hub kinds, but the real API/CLI radar search still returns only the Space because its identity omits kind. A distinct-name control returns all three. Exact-head local CI passes 1,642 tests and Actions is green; current-main integration still has the already reported test conflict.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 13:52
@rudycelekli

rudycelekli commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

@ktwu01 @toby-bridges Pushed c16bf8f. Current main (569c581) is merged and both test blocks are retained. GitHub now reports the branch mergeable.

QueryService now keeps the latest observation and stable result key per native Hub kind, using the existing exact artifact identity from the primary URL while preserving the original upstream ID. The regression executes collection → deduplication → repeated snapshot writes → public query/CLI for short/long same-named repositories and distinct-name controls. It also checks that a later dataset update preserves its model/Space siblings and their keys.

All six required local CI commands pass in a fresh checkout with its own Python 3.12 environment, Node 22, Bash 5 and the pinned recursive submodule: 1,789 tests passed, with 3,217 catalog records across five sources after regeneration. The upstream Actions run also passed on this exact head; the new Copilot review reports approval recommended with no findings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused identity changes are consistent across the pipeline and covered by comprehensive regressions.

Review effort: Balanced
Findings: None

@ktwu01
ktwu01 merged commit 4d4cca0 into ktwu01:main Oct 8, 2026
1 check passed
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.

4 participants