[Feature] Add reviewed PwC DrugBank protocol adapter

[yananlong]

Summary

This PR adds a manual-only adapter that converts a local Papers With Code DrugBank custom-format PostgreSQL dump into EEE aggregate evaluation logs. A reviewed YAML manifest binds each selected source cell to canonical model, metric, benchmark, split, protocol, and provenance metadata.

The manifest distinguishes transductive (both drugs seen), inductive-s1 (both drugs unseen), and inductive-s2 (one drug unseen), following the split convention described by the DSN-DDI primary source. The adapter preserves those split-qualified scores so downstream comparisons can show when model rankings or performance change across the protocols.

Design and safety

The adapter parses the dump strictly and fails closed on hash drift, missing or ambiguous anchors, malformed scores, incompatible metric scales, unexpected COPY rows, duplicate YAML keys, duplicate metric selections, conflicting benchmark protocols, coercive source IDs, or output-path violations. Conversion is atomic, so no output is published unless the complete bundle passes pre-publication schema and path validation.

Aggregate and result identities include the selected metric results, full reviewed metric configuration, and protocol semantics. The output records split_id, the generalization regime, entity-overlap evidence, and the protocol digest. It preserves DrugBank as the trusted source_data value while retaining the raw Papers With Code URL and archive URL as provenance metadata.

Scope and follow-up

No production qualification manifest or generated datastore records are included. The adapter remains unscheduled until a reviewed manifest and the corresponding downstream data contribution are prepared. The adapter preserves manifest-supplied scores and evidence; it does not independently verify cited source content or train/test membership from the aggregate dump, and it does not compute a performance delta between splits.

Verification

• The full test suite passes with 851 passed, 20 skipped.
• Focused adapter and catalog tests pass with 202 tests.
• Ruff, changed-file formatting, and whitespace checks pass; no dependency or lockfile changes were made.
• An integration test exercises a synthetic pgdumplib-generated custom-format dump, runs the adapter CLI, and separately passes uv run python -m every_eval_ever validate with zero warnings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[borgr]

@yananlong this rebases mechanically everywhere except one spot, and that spot is a design question I do not want to answer inside a conflict marker.

main now carries the Papers with Code adapter from #209: every_eval_ever/adapters/paperswithcode/, collection paperswithcode, converting the whole postgres dump from the huggingface/paperswithcode-backups bucket weekly, with a paperswithcode = ["pgdumplib>=4.0.0"] extra and a vendored eval-card-registry snapshot for metric bounds.

This PR adds a second package over the same dump — paperswithcode_drugbank, collection paperswithcode-drugbank, extra paperswithcode-drugbank = ["pgdumplib>=4.0.0,<5.0.0"]. The conflict is both sides editing the extras block and all, and resolving it either way encodes an answer:

1. Two packages, one extra. Drop the paperswithcode-drugbank extra, depend on paperswithcode, keep the packages separate. Cheapest, and leaves two adapters reading one dump.
2. One adapter, a drugbank mode. Fold this in as a mode of adapters/paperswithcode/, which is where the dump reading, the bucket resolution and the registry snapshot already live.

Also worth settling before either: whether the two overlap on rows. If the general adapter already emits DrugBank protocol leaderboards, the same source rows land in two collections under two identities, and that is a dedup problem rather than a packaging one.

Which do you want? Once that is decided the rest is a uv lock regeneration and a one-line table row, and I will do that part.

The <5.0.0 cap is the other small difference from main's specifier — intentional, or just what was current when you wrote it?

[borgr]

Rather than leave this on you: #269 continues these four commits with main merged in, the extras conflict resolved, and the overlap question answered rather than left open.

Short version. The duplicate pgdumplib extra now depends on main's paperswithcode one. And the overlap with the general adapter is joinable, not silent — both sides already record the PwC evaluations row id as additional_details.pwc_evaluation_id, so I documented that and the key in the docstring and the README row instead of restructuring anything. 951 tests pass, ruff clean, and it is labelled auto-review since this PR never got a pass (Copilot hit its quota).

Your <5.0.0 cap on pgdumplib is the one thing I dropped — if that was deliberate rather than what was current, say so and it goes back.

Leaving this open until #269 lands, so the work is never in limbo. Happy to hand the commits back if you would rather land it from your fork.

[borgr]

Pushed to this branch (c4d0790b5) rather than opening a second PR. @yananlong's four commits are still the base of the history, with two on top.

main merged in. The only conflict was the extras block: main gained the general Papers with Code adapter (#209) over the same postgres dump, with paperswithcode = ["pgdumplib>=4.0.0"]. This branch declared paperswithcode-drugbank = ["pgdumplib>=4.0.0,<5.0.0"] — the same package a second time. It now depends on the existing extra, and the two uv run --extra lines point at it. @yananlong, the <5.0.0 cap is the one thing that went: deliberate, or just what was current? It comes back in a line if deliberate.

The overlap, stated rather than left implicit. The scheduled paperswithcode run is --all --best-effort, so it converts every dataset in the dump — the DrugBank cells included, without the reviewed split and protocol semantics this adapter adds. So the same measurement appears in two collections.

That is joinable, not silent: both adapters record the PwC evaluations row id as additional_details.pwc_evaluation_id (paperswithcode/adapter.py:884 here, paperswithcode_drugbank/adapter.py:692 there), so a consumer reading both can tell one measurement from two. The second commit says so in the adapter docstring and the adapters/README.md row. What was missing was the statement, not the key.

If maintainers would rather have one adapter with a drugbank mode, or have the general adapter skip the reviewed cells, that is a larger change and belongs with #256's dedup discussion rather than in this PR.

uv run pytest tests 951 passed, 32 skipped; uv run ruff check clean. The regenerated uv.lock has a zero-line diff against main.

Labelled auto-review: this PR never got a review pass — Copilot hit its quota — so that would be a first read, not a re-read.

[borgr]

Review — one bug, one divergence, two smaller notes. No blockers on the protocol modelling, which is careful.

1. The uncertainty is parsed, then dropped and left on the wrong scale. parse_metric_value pulls ± out of a source string, _build_result sets score_details.uncertainty=None, and puts the raw figure in details.reported_uncertainty unscaled while the score is multiplied by scale_factor. Reproduced on this head:

'91.2% ± 1.4'  -> score=0.912, details.reported_uncertainty='1.4'
'62.65 ± 0.31' -> score=62.65 (or 0.6265 with source_scale: percent), reported_uncertainty='0.31'

A reader of the second field sees a spread wider than the score's whole range. No test covers this path — uncertainty and ± appear nowhere in the 1366-line test file, which is why it survived.

Scaling it is unambiguous. Putting it in the typed field is not: PwC's ± may be a standard error, a standard deviation or a CI half-width, and ScoreDetails.uncertainty.standard_error would assert one. Cleanest is an overlay field declaring the dispersion kind for the reviewed cells, then fill the typed field; failing that, scale the number and name the field for what it is.

2. Bounds and direction come from the overlay, while the sibling adapter over the same dump takes them from the registry. adapters/paperswithcode resolves min_score/max_score/lower_is_better/score_type against a vendored registry snapshot and fails closed on an unknown metric; here they are hand-reviewed YAML fields. Same dump, same metric, two possible answers — and the overlap means both adapters can publish the same PwC row. Suggest resolving through the same snapshot and keeping the overlay for the protocol semantics only, which is what it is uniquely good at.

3. export splits model_info.id by hand. log.model_info.id.split('/', 1) raises on a flat id, and bypasses datastore_path_components, which is the documented way to derive those components (#271 §path). valid_model_id currently guarantees a slash, so this is latent rather than live.

4. Fail-fast with no accounting is a choice worth stating. Every rejection raises, so one malformed cell aborts the run and nothing records exclusions. For a small hand-reviewed manifest that is defensible — unlike a scrape — but SourceConversionResult is what the skill asks for, so say in the docstring that the manifest is the ledger.

@yananlong — (1) needs a decision from you about what the ± is; the rest I can push if you'd rather not.

[borgr]

Fixed the two that were unambiguous, in 806087e53 — no point leaving them as your to-do when the answer is not in question.

Uncertainty scaling. parse_metric_value now returns a validated number rather than text, rejecting a negative or non-finite one, and _build_result applies the same scale factor it applies to the score. '99.49 ± 0.31' on a percent-scale metric now records score=0.9949 with reported_uncertainty=0.0031, and raw_value still holds the source's own spelling of both figures. Four tests cover the path, which had none — percent scale, identity scale, no dispersion, and three unusable values.

The path split. export uses datastore_path_components instead of model_info.id.split('/', 1), so a flat id can no longer raise there and the developer folder comes from the one place that owns that rule.

957 passed, 32 skipped; ruff clean.

Still yours, and still a real question: score_details.uncertainty stays unset. Filling standard_error would assert that PwC's ± is a standard error, and it may be a standard deviation or a CI half-width depending on the paper. If the reviewed cells all have a known dispersion kind, an overlay field is the clean way to say so and then the typed field can be filled. Happy to add it if you tell me what the manifest should carry.

The registry-vs-overlay question for bounds (finding 2) I have deliberately not touched — that one is a design call about two adapters over one dump, not a fix.

[borgr]

Follow-up in 421d495a0, and an answer to my own question about the ±.

It is not private knowledge, and we already had the answer. adapters/paperswithcode reads the same dump and documents the same conclusion independently — "PwC's '±' does not identify the spread as a standard error, a standard deviation, or a CI half-width, so the caller keeps it verbatim rather than coercing it into a typed Uncertainty." So leaving score_details.uncertainty unset is not a gap here, it is the house rule, and this PR already matched it. Nothing to decide.

What is knowable per cell is narrower: your overlay carries protocol_evidence_url and source_locator, so for a hand-reviewed manifest of a handful of cells, whoever reads the paper can record what its ± is. Worth doing later if the typed field matters; not a blocker.

Comparing the two adapters did turn up something, though: the same defect on main. The general adapter rescales scores onto the registry's canonical scale and leaves the ± verbatim, so a published record reads score: 0.337 beside reported_uncertainty: "0.82". #276 fixes that, and settles the convention — reported_uncertainty is what the source printed, reported_uncertainty_canonical is the same number on the score's scale, present only when a rescale applied.

421d495a0 moves this PR onto that convention, so the same key does not mean verbatim in one adapter and canonical in the other. 957 passed, 32 skipped; ruff clean.