feat(structure-viewer): add ted resource link

[FlorinSenoner]

Summary

• add a TED resource link beside UniProt and InterPro in the structure viewer header
• normalize the protein identifier to its base accession and encode it for the TED URL
• add focused helper coverage and a real custom-element regression test
• document the behavior through the OpenSpec change add-ted-link

Root cause

The structure viewer header had URL builders and rendered anchors only for AlphaFold, UniProt, and InterPro. There was no TED URL builder or TED anchor, so the resource could never appear.

Reproduction

1. Open /explore.
2. Search for and select protein A0A023VZF1.
3. Inspect the structure viewer header.

Before the fix, the header showed UniProt and InterPro but no TED link. After the fix, TED appears and targets https://ted.cathdb.info/uniprot/A0A023VZF1 in a safe new tab.

Tests

• focused TDD regression run: 17/17 tests pass
• pnpm --filter @protspace/core test:ci: 1,388 tests pass
• pnpm test:ci: pass
• pnpm precommit: pass
• strict OpenSpec validation: pass
• browser reproduction: exact TED URL, target blank, rel noopener noreferrer, no console errors

Closes #344

[tsenoner]

Automated review

Does it solve #344? Yes — the PR adds buildTedUrl() in packages/core/src/components/structure-viewer/header-links.ts, reusing the same getBaseAccession() + encodeURIComponent() normalization as the UniProt and InterPro builders, and renders a fourth .header-link anchor with matching target/rel attributes. It reaches real users: apps/web/src/pages/Explore.tsx sets show-header="true", so the header renders on every protein selection, and buildTedUrl('W6JQJ9.2') produces exactly the URL from the issue's example. Nothing the issue asked for is missing; the only gap is peripheral documentation.

Found 1 issue:

1. Two user-facing docs pages enumerate the structure-viewer header links by name and still say AlphaFold, UniProt and InterPro, so they now contradict the shipped UI — docs/explore/structures.md:12 and docs/explore/index.md:60 are closed enumerations, not examples, so a reader sees a fourth link the docs say does not exist, with no explanation of what TED is (even though docs/guide/annotations.md already describes it). This is a one-line addition on each page; the same drift happened for the sibling feature, where the InterPro link commit (2bbff9a) left both sentences stale and a separate docs pass (f854a36) had to fix them four days later. The shared screenshot docs/explore/images/structure-viewer.png embedded on both pages also predates the change and can be regenerated with pnpm docs:images.

protspace/docs/explore/structures.md

Lines 11 to 13 in ef9b3dc

	1. The structure viewer appears in the sidebar below the legend
	2. Links to [AlphaFold Database](https://alphafold.ebi.ac.uk/), [UniProt](https://www.uniprot.org/), and [InterPro](https://www.interpro.org/) appear at the top - click them anytime
	3. The AlphaFold structure file is fetched directly from the [AlphaFold Database API](https://alphafold.ebi.ac.uk/api-docs); the [3D-Beacons API](https://www.ebi.ac.uk/pdbe/pdbe-kb/3dbeacons/) is used only to look up the model page link

Deliberately not flagged: the TED anchor rendering unconditionally (it mirrors the pre-existing AlphaFold/UniProt/InterPro anchors, and design.md records the decision to match existing behaviour rather than add availability checks) and the repeated anchor markup (design.md explicitly rejects a resource-descriptor array as scope creep here).

🤖 Generated with Claude Code

_{Reviewed at ef9b3dc against issue #344.}

[FlorinSenoner]

Disposition: actionable and needs to be addressed.

Verified against current head ef9b3dc2ca2dc6847557f53177f8df5c4d4c188d: structure-viewer.ts renders TED, while docs/explore/structures.md:12 and docs/explore/index.md:60 still enumerate only AlphaFold, UniProt, and InterPro. Both pages embed docs/explore/images/structure-viewer.png; the checked-in screenshot visibly omits TED, and the documented pnpm docs:images pipeline regenerates it.

Required direction: update both Explore descriptions to include/link TED and regenerate the shared structure-viewer screenshot. No implementation was performed as part of this triage.

[FlorinSenoner]

Implemented the verified documentation feedback in 569e67dd975c1018d43d1785a5d9ce4bcda9d708: both Explore pages now link TED, the shared structure-viewer screenshot was regenerated and visually verified with the TED link present, and stale screenshot-pipeline readiness probes were updated for the current scatterplot/controller internals. Verification passed with pnpm docs:images (13 static screenshots, 7 animations, 7 GIF conversions), openspec validate add-ted-link --strict, and pnpm precommit.

[tsenoner]

Adversarial review

Reviewed in an isolated worktree by three independent lenses (code quality, adversarial correctness, issue-resolution audit), with every finding then put through a refuter whose default position was that it is a false positive. 4 raised, 2 survived refutation.

Applied, then reverted (7f7ca7ee)

I initially applied the waitForFunction argument-position fix in scripts/docs-screenshots/helpers.ts,
then reverted it. An adversarial confirm pass proved it is not behavior-preserving, so it belongs to
you rather than to an automated cleanup pass. The branch is back to its original state; details are in
the findings below.

Issue resolution — resolves the issue

The triage is correct and the PR description is honest — unusually so. Nothing was narrowed,
redirected, or declined. I independently checked the two claims that could have been hand-waving:

1. "There was no TED URL builder or TED anchor, so the resource could never appear" — confirmed
   against origin/main; header-links.ts had only AlphaFold/UniProt/InterPro builders.
2. The "stale image-pipeline readiness checks that blocked regeneration" claim (openspec
   proposal.md:11) is real, not an excuse for scope creep. scripts/docs-screenshots/helpers.ts:55
   used Array.isArray(plot._plotData), but _plotData is typed PlotData
   (packages/core/src/components/scatter-plot/scatter-plot.ts:142), and PlotData is a struct-of-
   arrays interface with a readonly length field (packages/utils/src/types.ts:125-131) — never an
   array. So waitForDataLoad could never return true and every screenshot capture timed out. The one-
   line fix to plot._plotData?.length was genuinely required to regenerate structure-viewer.png
   (which comes from capture-static.spec.ts:207-241, a consumer of that shared helper).
   "Closes [FEATURE] Add TED link #344" is warranted. The issue is a single, fully-specified feature request with no comments
   expanding its scope (I fetched it: gh issue view 344 shows an empty comments array), the requested
   URL shape is implemented exactly, and the user-visible outcome is demonstrated in the committed
   screenshot. I would not downgrade this to "Related to".
   The one place the PR overreached without saying so in the PR body: the capture-animations.spec.ts
   rewrite (see gaps).

Gaps found by the issue audit (5)

• Scope creep in scripts/docs-screenshots/capture-animations.spec.ts (+47/-40) that this PR cannot verify. Only the one-line helpers.ts:55 fix was needed to regenerate structure-viewer.png (produced by capture-static.spec.ts:207). The animations spec changes rewire the duplicate-stack probes from the removed _duplicateStackByKey / _expandedDuplicateStackKey to _dupOverlay.byKey / _dupOverlay.expandedKey, add a forced PCA-projection selection that throws if absent, and switch a waitForFunction to explicit polling: 200.
  • Why it matters: These probes drive the duplicate-badge GIF, which this PR does not regenerate — the only image changed is structure-viewer.png. The new field paths are runtime-reachable (TypeScript private is soft-private; I confirmed byKey at duplicate-stack-overlay-controller.ts:79 and expandedKey at :81, and _dupOverlay at scatter-plot.ts:241), so the fix is plausible — but it ships unexercised. If the projection-index or polling change is subtly wrong, the breakage surfaces on some future unrelated docs regeneration, and the blame will point at that PR rather than this one. It also makes this PR's diff larger than its stated purpose.
  • Suggested follow-up: Either split the capture-animations.spec.ts repair into its own PR that actually runs the animation captures and commits the regenerated GIF, or run the duplicate-badge capture once on this branch and note the result in the PR body so the change is not merged untested.
• TED coverage is narrower than UniProt/InterPro, but the link is rendered unconditionally for every proteinId. getBaseAccession only strips a .-suffix, so an isoform ID like P12345-2 or a non-UniProt dataset ID (my_seq_001, sp|P12345|NAME) is passed straight through and produces a dead TED URL.
  • Why it matters: docs/guide/faq.md:50 already tells users "Structures load automatically from AlphaFold if your protein IDs are UniProt accessions", i.e. non-UniProt datasets are an expected case. TED is derived from the AlphaFold DB, so its per-accession coverage is strictly narrower than UniProt's and somewhat narrower than InterPro's — the dead-link rate for the new TED link will be visibly higher than for the two links it sits beside. The design doc (design.md:37) consciously accepts this ("let the external service report availability"), which is defensible and consistent with existing behavior, so this is a follow-up rather than a blocker.
  • Suggested follow-up: Open a follow-up to handle unresolvable resource links uniformly — e.g. suppress or visually de-emphasize the UniProt/InterPro/TED row when _loadStructure reports No 3D structure was found for <id> (structure-viewer.ts:182), since that error is already a strong signal the accession is not in the AlphaFold DB and therefore not in TED.
• .header-links is display: flex with no flex-wrap (packages/core/src/components/structure-viewer/structure-viewer.styles.ts:73-77); only the outer .header-info sets flex-wrap: wrap (:61-66). The design doc (design.md:39) states the change "Reuse[s] the existing wrapping flex container", which is inaccurate about the container the three links actually live in.
  • Why it matters: The link row grew from two labels to three plus an extra separator and can now only wrap as a whole unit, not between links. At the app's default right-panel width it fits — the regenerated screenshot proves that — but a narrower panel or a longer accession would overflow the row horizontally rather than wrap. There is no regression test or screenshot covering a narrow/mobile layout, so this would only be caught by eye.
  • Suggested follow-up: Add flex-wrap: wrap to the .header-links rule (a one-line, zero-risk change), and correct the design.md sentence so the accepted trade-off is recorded against the container that actually applies.
• docs/explore/control-bar.md:39 still describes the annotation dropdown sections as "(UniProt, InterPro, Taxonomy, Other)", while packages/core/src/components/control-bar/annotation-categories.ts:9 defines 'Biocentral' | 'InterPro' | 'TED' | 'Taxonomy' | 'UniProt' | 'Other'.
  • Why it matters: Pre-existing staleness, not introduced here and genuinely outside issue [FEATURE] Add TED link #344's scope (it concerns annotation grouping, not resource links). Flagging it only because it is a TED-specific doc gap that this PR walked directly past while updating the two other TED-mentioning Explore docs — a reader comparing the docs will see TED acknowledged in one place and missing in another.
  • Suggested follow-up: Fold the one-line list update into this PR (it is already touching docs/explore/) or file a small docs: issue to resync control-bar.md with annotation-categories.ts.
• The OpenSpec change is not archived: the new protein-resource-links capability exists only under openspec/changes/add-ted-link/specs/, never merged into openspec/specs/ (which currently holds only bundle-format-contract, prep-failure-routing, prep-observability, support-contact).
  • Why it matters: AGENTS.md states openspec/specs/ is "the source of truth for current behavior", so until /opsx:archive runs, the shipped resource-link behavior is not represented in the source of truth. Low severity: five other changes in the repo (add-eat-visualization, unified-visibility-model, optimize-e2e-suite, …) are likewise unarchived, so the repo's actual practice is clearly to archive on a separate cadence — this is consistent, not a deviation.
  • Suggested follow-up: No action needed for this PR; run /opsx:archive add-ted-link in the next batch archive pass so protein-resource-links lands in openspec/specs/.

Findings needing a decision (2)

These were left for you rather than auto-applied: each changes behavior, needs a product call, or reaches outside this diff.

1. waitForDataLoad/waitForLegend pass their waitForFunction options in the arg position, so the timeout and polling they compute are silently discarded.

scripts/docs-screenshots/helpers.ts:55 · low · correctness · applied then reverted

Playwright's signature is waitForFunction(pageFunction, arg, options) (verified in playwright- core@1.61.0/types/types.d.ts:5880) — there is no arity sniffing. helpers.ts:50-60, :64-67 and
:78-86 pass { timeout, polling: 200 } as the second positional argument, so it is serialized as
the (unused) page-function argument and the real options object is undefined. This PR fixed
exactly this mistake in the sibling file (capture-animations.spec.ts:564-575 now passes
undefined, { timeout: 10_000, polling: 200 }) but left it in the call it edited. Concrete failure:
await waitForDataLoad(page, 5000) does not fail after 5s — actionTimeout is unset in
playwright.config.ts, so the wait runs unbounded until the 60s test timeout and reports an opaque
"Test timeout of 60000ms exceeded" instead of a waitForFunction failure naming the readiness
predicate. This is exactly how the now-fixed Array.isArray(plot._plotData) staleness manifested.
polling: 200 is likewise ignored (defaults to raf).

Why it was reverted rather than kept: playwright.config.ts leaves actionTimeout unset (default
0), so all three waits were previously unbounded, capped only by the 60s per-test timeout. Applying
the fix caps them at 30s/30s/15s and moves polling from raf to fixed 200/100/200ms intervals. A probe
spec confirmed the old form never times out while the new form rejects on schedule. That is almost
certainly what you intended when you wrote { timeout, polling } — but it can turn a slow-but-successful
screenshot capture into a hard failure, so it is your call, not an automated one. Blast radius is small:
docs:screenshots/docs:animations/docs:images are referenced by no workflow in .github/workflows/,
so this runs only on manual local invocations.

The fix, if you want it

In /Users/tsenoner/Documents/projects/protspace-suite/.protspace-wt-pr402/scripts/docs-
screenshots/helpers.ts, move the options object into the third argument position by inserting
undefined, before it at the three call sites named in the finding:

1. line 50-60 (waitForDataLoad readiness predicate): change the trailing },\n { timeout, polling: 200 },\n ); to },\n undefined,\n { timeout, polling: 200 },\n );
2. line 64-67: await page.waitForFunction(() => !document.getElementById('progressive-loading'), {\n timeout,\n polling: 100,\n }); becomes await page.waitForFunction(\n () => !document.getElementById('progressive-loading'),\n undefined,\n { timeout, polling: 100 },\n );
3. line 78-86 (waitForLegend predicate): change },\n { timeout, polling: 200 },\n ); to },\n undefined,\n { timeout, polling: 200 },\n );
   This is exactly the form already used at capture-animations.spec.ts:564-575 and at helpers.ts:100
   (waitForWebGLContext), so it type-checks. Note for a follow-up (do not autofix in this pass):
   helpers.ts:141, :156 and :175 have the identical misplacement, and fixing :156/:175 is what restores
   their intended try/catch fallbacks; the same pattern also appears in capture-static.spec.ts.

2. Forcing plot.selectedProjectionIndex directly bypasses the control bar, so the recorded duplicate-badges.gif shows a projection dropdown that contradicts the plotted data.

scripts/docs-screenshots/capture-animations.spec.ts:554 · medium · correctness

Concrete failure: apps/web/public/data.parquetbundle (the dataset /explore loads) has
projections ['ProtT5 — UMAP 2', 'ProtT5 — PCA 2', 'ESM2-650M — UMAP 2', 'ESM2-650M — PCA 2'], and
data-renderer.ts:94 starts the plot at index 0. The new code computes pcaIndex = 1 and assigns
plot.selectedProjectionIndex = 1 on the scatterplot element. The control bar only mirrors that
value inside _syncWithScatterplot(), which control-bar.ts:1233-1237 schedules exactly once via
setTimeout(..., 50) at auto-sync setup; a selectedProjectionIndex change does not dispatch
data-change (scatter-plot.ts:764-782 only emits on data / filteredProteinIds /
filtersActive), so nothing re-syncs it. Result: the plot renders ProtT5 — PCA 2 while the
control-bar trigger (control-bar.ts:564, which renders this.selectedProjection) still reads
ProtT5 — UMAP 2. The animation project records the full 1536x864 viewport
(playwright.config.ts), so the control bar is in frame and the published
docs/explore/images/duplicate-badges.gif would ship that contradiction. The switch also appears
unnecessary: duplicate-badges.gif was successfully generated on the default projection in 68c3bd9
(2026-05-11), before the _duplicateStackByKey rename broke the probe, and duplicate grouping is
cross-projection (0c6e079). The GIF is not regenerated in this PR, so this path is unverified.

Suggested fix

In /Users/tsenoner/Documents/projects/protspace-suite/.protspace-wt-pr402/scripts/docs-
screenshots/capture-animations.spec.ts, replace the body of the first page.evaluate in the
duplicate-badges.gif test (lines 542-558) with a control-bar-driven selection so the label and the
plotted data stay in sync:

await page.evaluate(async () => {
  const plot = document.querySelector('#myPlot') as DuplicatePlotProbe | null;
  const controlBar = document.querySelector('#myControlBar') as
    | (HTMLElement & { applyProjectionSelection(projection: string): void })
    | null;
  if (!plot || !controlBar) {
    throw new Error('Duplicate-badge capture needs #myPlot and #myControlBar');
  }
  const pca = plot.data?.projections?.find((projection) => projection.name.includes('PCA'));
  if (!pca) {
    throw new Error('Duplicate-badge capture requires a PCA projection');
  }
  controlBar.applyProjectionSelection(pca.name);
  await plot.updateComplete;
  plot.config = { ...(plot.config ?? {}), enableDuplicateStackUI: true };
  await plot.updateComplete;
});

applyProjectionSelection (control-bar.ts:139) sets selectedProjection, mirrors the index onto
the scatterplot while autoSync is on (data-renderer.ts:209 turns it on after load), and dispatches
projection-change so the app's view controller/URL stay consistent.
DuplicatePlotProbe.selectedProjectionIndex can then be dropped from the type at line 17 if unused.
Do not autofix: the capture must be re-run and duplicate-badges.gif regenerated to confirm PCA still
yields a multi-point stack before this is trusted.

2 further finding(s) were raised and refuted during verification.

[FlorinSenoner]

Addressed the adversarial review in comment 5203113387 on 52b8afc3f40313e6dd6377a4a510af5ef292b9ab.

1. Animation-script scope: kept. pnpm docs:images is the documented canonical image pipeline and includes the animation project. A live probe of the current bundled dataset found 0 multi-point stacks on the default ProtT5 UMAP projection and 265 on ProtT5 PCA, so the PCA switch is required for this capture rather than incidental scope. The focused duplicate-badge capture now passes and the GIF was regenerated.
2. TED availability for non-UniProt IDs: no code change. openspec/changes/add-ted-link/design.md explicitly makes availability checks a non-goal and records deterministic-link parity with UniProt/InterPro; changing that policy would be a separate product change.
3. Header wrapping: no code change. .header-info is the existing wrapping container named by the design, while .header-links contains only fixed link labels; a longer accession is a separate .protein-id flex item and does not widen the link row. Styling changes are also an explicit non-goal for this change.
4. Control-bar category docs: no change. The stale annotation-category list predates this PR and concerns annotation grouping, not the resource-link capability in [FEATURE] Add TED link #344.
5. OpenSpec archival: no change. The change remains active while the PR is open; archive/merge into openspec/specs/ belongs to the post-merge archive pass.
6. waitForFunction options: fixed at the three reviewed call sites by passing undefined as the page-function argument. Added real Playwright regressions for readiness, loading-overlay, and legend timeouts. RED: each 100 ms timeout ran until the 2,000 ms test timeout; GREEN: all 3 pass.
7. Projection/control-bar mismatch: fixed by selecting PCA through controlBar.applyProjectionSelection() and asserting the plotted projection equals the visible control-bar selection before recording. RED: the assertion timed out with direct plot mutation; GREEN: the complete 21-action duplicate-badge capture passed, and the regenerated GIF visibly labels ProtT5 — PCA 2.

Verification: focused helper tests 3/3; focused duplicate-badge Playwright capture 1/1; GIF conversion 1/1; openspec validate add-ted-link --strict; staged pnpm precommit (ESLint/Prettier, typecheck, Knip, annotation-doc check, docs build). Exact-head checks: Code Quality passed, Build Documentation passed, Bundle format contract passed, E2E path-filter skipped.