Skip to content

fix(stage6): canonical NeuroScape URLs + mirror link-check to PR preview#10

Merged
satra merged 1 commit into
mainfrom
fix/refs-link-check
May 17, 2026
Merged

fix(stage6): canonical NeuroScape URLs + mirror link-check to PR preview#10
satra merged 1 commit into
mainfrom
fix/refs-link-check

Conversation

@satra

@satra satra commented May 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Restored the two NeuroScape references that I had guessed at and that 404'd on the production deploy:

    • Repo: github.com/ccnmaastricht/NeuroScape (Cognitive Computational Neuroscience Maastricht — not sensein/neuroscape as I'd guessed).
    • Paper: apertureneuro.org/article/156380-the-evolving-landscape-of-neuroscience ("The evolving landscape of neuroscience"). Both HEAD-verified 200.
  • Root cause of why this didn't surface in the original PR's CI: deploy-ui.yml ran the link check, pr-preview.yml didn't. Now both workflows run it on the same paths (site/** plus specs/008-ui-rewrite/contracts/references.yaml plus src/ohbm2026/ui_data/link_check.py), so a broken citation will fail the PR check pre-merge instead of waiting until production deploy.

Test plan

  • HEAD-checked both restored URLs locally (200)
  • link_check unit tests still pass (8/8)
  • PR-preview workflow runs the new link-check step against this branch
  • After merge, deploy-ui.yml succeeds and the production site shows the new build SHA

🤖 Generated with Claude Code

Two parts:

1. The post-merge deploy failed link-check because I'd guessed the
   NeuroScape URLs earlier. Web search confirmed the canonical ones:
     * github.com/ccnmaastricht/NeuroScape (Cognitive Computational
       Neuroscience Maastricht — NOT sensein, which was my guess).
     * apertureneuro.org/article/156380-the-evolving-landscape-of-neuroscience
       ("The evolving landscape of neuroscience").
   Both HEAD-verified 200. Restored the references.yaml entries and
   the about-page object literal that references them.

2. The reason this only surfaced post-merge: deploy-ui.yml runs the
   link checker, pr-preview.yml didn't. Mirrored the same step into
   pr-preview.yml so a broken citation fails the PR check BEFORE merge
   instead of post-deploy. Both workflows now also trigger on changes
   to specs/008-ui-rewrite/contracts/references.yaml and
   src/ohbm2026/ui_data/link_check.py so the gate doesn't get bypassed
   on doc-only edits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@satra
satra temporarily deployed to pr-preview-10 May 17, 2026 22:25 — with GitHub Actions Inactive
@satra
satra merged commit af2fb19 into main May 17, 2026
2 checks passed

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the NeuroScape repository and paper references in both the Svelte about page and the YAML specification file. Review feedback identifies inconsistencies in citation formatting between files, the use of a GitHub handle instead of a full name for authors, and unconventional citation practices involving organization names.

Comment on lines +47 to +53
title: 'NeuroScape — code repository (ccnmaastricht)',
url: 'https://github.com/ccnmaastricht/NeuroScape'
},
neuroscape_paper: {
title:
'Vali Tehrani et al. (2024)NeuroScape: a domain-specific embedding for neuroscience abstracts (Aperture Neuro)',
url: 'https://apertureneuro.org/article/124574-neuroscape-a-domain-specific-embedding-for-neuroscience-abstracts'
'The evolving landscape of neuroscience — Aperture Neuro',
url: 'https://apertureneuro.org/article/156380-the-evolving-landscape-of-neuroscience'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The titles for the NeuroScape references are inconsistent with the format used for other academic citations in this file (e.g., lines 23, 27, 31, 35), which include the author and year in the string. Additionally, these titles are not synchronized with the corresponding title fields in specs/008-ui-rewrite/contracts/references.yaml. Since these title properties appear to be unused in the Svelte template, they should either be updated for consistency or removed to reduce duplication.

title: 'NeuroScape — code repository'
authors: sensein
title: 'NeuroScape — code repository (ccnmaastricht / Cognitive Computational Neuroscience Maastricht)'
authors: ccnmaastricht

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The authors field uses a GitHub handle (ccnmaastricht) instead of a proper name, which is inconsistent with other entries in this file (e.g., "Oxford Abstracts Ltd.", "Microsoft Research").

    authors: Cognitive Computational Neuroscience Maastricht

title: 'NeuroScape: a domain-specific embedding for neuroscience abstracts'
authors: Vali Tehrani et al.
title: 'The evolving landscape of neuroscience (Aperture Neuro)'
authors: Cognitive Computational Neuroscience Maastricht et al.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using an organization name followed by "et al." is unconventional for scientific paper citations. This field should ideally list the primary human authors of the paper (e.g., "Vali Tehrani et al.") to maintain standard citation practices.

satra added a commit that referenced this pull request May 17, 2026
Addresses the gemini-code-assist review comments on PRs #9 and #10
plus the axe-core a11y audit results from the live production site.

PR #9 review fixes:
* state_key.discover_rollup_state_key — docstring previously said
  "picks the most recent by mtime" but the implementation raises on >1
  match; corrected the docstring (intentional design: no implicit
  newest-wins, the operator must disambiguate).
* PR #9 also flagged a cross-device rename OSError in builder.py and
  redundant inline `shutil` imports. Both already addressed by the
  earlier in-place-write refactor — no shutil/tempfile/rename calls
  remain in builder.py. Verified.

PR #10 review fixes:
* references.yaml authors fields: `ccnmaastricht` → `Mario Senden`
  (the canonical author per the Aperture Neuro paper). Year corrected
  to 2026.
* Both entries' titles now follow the same `<Author> (<Year>) — <title>`
  format as the other references in the registry.

a11y (axe-core against production):
* `scrollable-region-focusable` was the one serious WCAG 2.1 violation
  surfacing on every route. Added `tabindex="0"` (+ `role="region"`
  + `aria-label` where useful) to every `overflow-y:auto` container:
  cluster-grid, related-list scroll, facet options scroll, cart items.
* New `site/src/tests/e2e/a11y.spec.ts` Playwright spec that runs the
  axe audit against the home / about / permalink routes. Reads
  `TARGET_BASE` so the same probe can hit production OR a PR preview.
* `@axe-core/playwright` added as a dev dep.

(beta) tag:
* Page <title> + the H1 in the header now read "OHBM 2026 Atlas (beta)"
  so users see clearly the site is pre-launch.

Empty-state tip:
* Dropped the `Models × inputs` and `Cells` lines from the right-pane
  "Tap an abstract to see its details here" affordance — they were
  more developer-facing than reader-facing. Just `Accepted abstracts`
  stays.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
satra added a commit that referenced this pull request May 17, 2026
…ate (#11)

Addresses the gemini-code-assist review comments on PRs #9 and #10
plus the axe-core a11y audit results from the live production site.

PR #9 review fixes:
* state_key.discover_rollup_state_key — docstring previously said
  "picks the most recent by mtime" but the implementation raises on >1
  match; corrected the docstring (intentional design: no implicit
  newest-wins, the operator must disambiguate).
* PR #9 also flagged a cross-device rename OSError in builder.py and
  redundant inline `shutil` imports. Both already addressed by the
  earlier in-place-write refactor — no shutil/tempfile/rename calls
  remain in builder.py. Verified.

PR #10 review fixes:
* references.yaml authors fields: `ccnmaastricht` → `Mario Senden`
  (the canonical author per the Aperture Neuro paper). Year corrected
  to 2026.
* Both entries' titles now follow the same `<Author> (<Year>) — <title>`
  format as the other references in the registry.

a11y (axe-core against production):
* `scrollable-region-focusable` was the one serious WCAG 2.1 violation
  surfacing on every route. Added `tabindex="0"` (+ `role="region"`
  + `aria-label` where useful) to every `overflow-y:auto` container:
  cluster-grid, related-list scroll, facet options scroll, cart items.
* New `site/src/tests/e2e/a11y.spec.ts` Playwright spec that runs the
  axe audit against the home / about / permalink routes. Reads
  `TARGET_BASE` so the same probe can hit production OR a PR preview.
* `@axe-core/playwright` added as a dev dep.

(beta) tag:
* Page <title> + the H1 in the header now read "OHBM 2026 Atlas (beta)"
  so users see clearly the site is pre-launch.

Empty-state tip:
* Dropped the `Models × inputs` and `Cells` lines from the right-pane
  "Tap an abstract to see its details here" affordance — they were
  more developer-facing than reader-facing. Just `Accepted abstracts`
  stays.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant