From 6b6785ee0f18e6184cad682067bf61433e20b484 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 16 Sep 2026 16:47:53 +0000 Subject: [PATCH 1/2] docs(specs): record why the interactor overlay is built the way it is The reasoning behind these choices lived only in commit messages, and nobody reads those in a year when they are wondering why a number is what it is. specs/002-interactor-overlay-decisions records eleven decisions, each with what was rejected and the measurement that settled it: the count being interactions rather than entities, the badge not being drawn below 0.6 zoom and the notice that replaces it, the download being the data rather than the view, the threshold being per-resource, a reader's own data being read in the browser, and the rest. Two things it deliberately does that the template does not ask for: - Every figure is attributed to a measurement and a date, because several of these decisions exist *because* a measurement contradicted an assumption, and a reader should be able to tell which. - Decisions still open are recorded as open -- where the control belongs, and the badge colour -- rather than omitted. An absent decision reads as an oversight; a recorded one reads as a choice nobody has made yet. D7 is the one to read first if anything in this area misbehaves: a reader's gesture and the address being replayed reach the same code, two released bugs came from not distinguishing them, and a third of the same shape is plausible. The custom upload contract is written down as measured rather than as documented, because it is not documented anywhere: the header the service wants, the third column it refuses, and the first pair it silently eats without one. Co-Authored-By: Claude Opus 5 --- .../checklists/requirements.md | 45 ++++ .../research.md | 230 ++++++++++++++++++ .../002-interactor-overlay-decisions/spec.md | 174 +++++++++++++ 3 files changed, 449 insertions(+) create mode 100644 specs/002-interactor-overlay-decisions/checklists/requirements.md create mode 100644 specs/002-interactor-overlay-decisions/research.md create mode 100644 specs/002-interactor-overlay-decisions/spec.md diff --git a/specs/002-interactor-overlay-decisions/checklists/requirements.md b/specs/002-interactor-overlay-decisions/checklists/requirements.md new file mode 100644 index 00000000..744885f4 --- /dev/null +++ b/specs/002-interactor-overlay-decisions/checklists/requirements.md @@ -0,0 +1,45 @@ +# Specification Quality Checklist: The interactor overlay, as built + +**Purpose**: Validate this record is usable by someone who was not here +**Created**: 2026-09-16 +**Feature**: [spec.md](../spec.md) · [research.md](../research.md) + +## Content Quality + +- [x] No implementation details in the spec itself — they are in research.md, + where a decision needs them to be understood +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders (spec.md; research.md is for + whoever maintains this next) +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic +- [x] All acceptance scenarios are defined, and each names the spec that holds it +- [x] Edge cases are identified — each is one a released bug actually hit +- [x] Scope is clearly bounded: this records shipped behaviour, not future work +- [x] Dependencies and assumptions identified + +## Record Quality + +This checklist adds items the standard one does not, because this document's +purpose is to be believed later. + +- [x] Every figure is attributed to a measurement and a date +- [x] Every decision names what was rejected, not only what was chosen +- [x] Decisions that match the old browser cite the source file +- [x] Open decisions are recorded as open (D10: the control's position, and the + badge colour) rather than omitted +- [x] Where a measurement contradicted an assumption, that is said + +## Notes + +- D7 is the one to read first if something in this area misbehaves: two released + bugs came from the same cause and a third of the same shape is plausible. +- The numbers here describe third-party data on a particular day. Tests assert + invariants rather than these figures, deliberately; do not "fix" a test to + match a number in this document. diff --git a/specs/002-interactor-overlay-decisions/research.md b/specs/002-interactor-overlay-decisions/research.md new file mode 100644 index 00000000..2fd4c5d0 --- /dev/null +++ b/specs/002-interactor-overlay-decisions/research.md @@ -0,0 +1,230 @@ +# Decisions behind the interactor overlay + +Each entry: what was decided, why, what was rejected, and the measurement that +settled it. Dates are when the figure was read off beta or the ContentService. + +--- + +## D1. The count beside a resource is interactions, not entities + +**Decision**: The number beside each resource counts **interactions** — the same +unit as the badge drawn on an entity. How many entities carry them is in the +tooltip. + +**Why**: It was entities, and the two were rendered identically with nothing +saying so. Reported from beta as impossible: Reactome-FIs read **15** beside an +MCM7 badge reading **17**. Both were correct and the pair was nonsense. + +**Measured** (2026-09-15, R-HSA-69306 with Reactome-FIs): 18 accessions posted, +13 entities returned, **78 interactions**, MCM7 alone **17**. The service's +`count` field equals the interactor array length throughout. + +**Rejected**: keeping entities and relabelling. Coverage is the more useful +signal when choosing between resources, but a bare number that cannot be compared +to the badge beside it is worse than a less useful number that can. Coverage +survives in the tooltip. + +--- + +## D2. A diagram tally counts each protein once + +**Decision**: When tallying what is drawn, badges are deduplicated by accession. + +**Why**: A protein drawn twice on a diagram carries two badges repeating the same +interactions. Summing badges therefore overstates. + +**Measured** (2026-09-15, R-HSA-69306): **15 badges over 13 accessions**; summing +badges gives 84 where the resource holds 78. The panel asks the resource directly +before anything is drawn, so it would have shown 78 and then changed its mind to +84 on the click. + +--- + +## D3. The count badge is not drawn below 0.6 zoom + +**Decision**: Below 0.6 the badge is not drawn at all. + +**Why**: It is 30 model units wide. At the **0.283** R-HSA-1368108 opens at, that +is **six screen pixels** holding a two-digit number. The old browser stops drawing +it too: `RendererManager.setFactor` swaps renderer tiers at 0.5 and +`ProteinRenderer000` never calls `drawSummaryItems`. + +**Rejected**: holding a minimum on-screen size as a map pin does — issue #200's +suggestion 3. A badge that keeps its size while the diagram shrinks detaches from +the entity it belongs to, and at the zooms in question would cover it. + +**0.6 rather than 0.5**: chosen here rather than copied, because a cytoscape zoom +and a GWT factor are not the same quantity. 0.6 puts the badge at 18 screen +pixels, which is where its digits stop being a smudge. + +--- + +## D4. If they cannot be seen, say so + +**Decision**: When a resource has badges and none can be drawn, a bar says so and +offers "Zoom to them". + +**Why**: D3 is right and left a hole. Measured (2026-09-15, R-HSA-1368108 at its +opening zoom of 0.283): choosing IntAct put **nine badges** on the graph, **none +visible**, and said nothing anywhere. The overlay appeared to do nothing — which +is the complaint this whole feature began with. + +**The button fits the badges' _entities_, not the badges.** `cy.fit` ignores +elements it cannot see, so fitting to hidden badges moved nothing: 0.283 before, +0.283 after. Fitting those nine entities reaches only 0.308, still below the +drawing threshold, so there is a floor just past it. + +--- + +## D5. The download is the data, not the view + +**Decision**: The file carries every interaction the opened entities hold, +whatever the threshold. + +**Why**: A spreadsheet can filter further; nothing recovers rows that were never +written; and a file of twelve rows beside a badge reading 73 is a contradiction +the reader has to resolve. Reading the drawn nodes also quietly lost one of +BHLHE40's 73 — an interaction whose partner is already on the diagram gets an +edge but no node of its own. The badge counts it; so should the file. + +It also means the drawing cap cannot truncate the file: draw 100 of 150 and the +file still carries 150. + +--- + +## D6. The threshold is remembered per resource + +**Decision**: Each resource opens at the threshold it was last left at; one never +seen opens at the default (0.45, the old browser's `DEFAULT_SCORE`). + +**Why**: Resources do not score alike, so one number across all of them is the +wrong shape. The old browser holds the same map — +`Map interactorsThreshold` in `InteractorsContent.java`. + +**Deliberately not in the URL**: the address carries the threshold _in force_, +which is what sharing needs. Putting all fourteen in it would make the address +unreadable for no one's benefit. + +**A resource never seen does not inherit.** Inheriting is how a reader ends up +with an empty diagram and no idea why. + +--- + +## D7. A reader's gesture and the address being replayed are different things + +**Decision**: The replay path says it is a replay. + +**Why**: Both reach the same method — `stateToDiagram` reads the address and +hands it to the same call a click goes through. Two released bugs came from not +distinguishing them: + +1. A shared threshold was reset on load. Measured: `?overlay=Reactome-FIs& +interactorScore=0.8` arrived and settled at no threshold at all, while the same + address _without_ the overlay kept 0.8. +2. The overlay was lost on Back, because the replay looked like the + toggle-to-unselect gesture. Measured by logging history writes: the browser + restored `?overlay=IntAct&tab=details` and the app immediately pushed + `?tab=details` over it. + +**This is the recurring fault in this area.** Any path where the URL is replayed +through a handler that also serves a reader's gesture is suspect. + +--- + +## D8. A reader's own data is read in the browser + +**Decision**: A file or pasted table is parsed in the page. No request is made. +Uploading is an explicit tick-box. + +**Why**: Uploading was inherited from the GWT browser, not required. Traced +(2026-09-15): the content was posted to the ContentService, parsed there, written +to `custom/.bin`, and named by a token that became part of the page's +address. A fresh upload was read back by a request carrying **no session**. The +store held **323 files, oldest 2019-05-01**, and nothing expires. + +**Measured**: adding a local resource went from **two** requests to **zero**. + +**Kept as a choice** because that token is the only thing that makes an overlay +survive a reload or open for a colleague. Removing it would have taken away a real +capability in the name of privacy; making it a choice gives the reader both. + +**URL and PSICQUIC options still go to the server**: a page cannot fetch an +arbitrary origin. + +--- + +## D9. Nothing is asked until the panel is open + +**Decision**: Resource counts are prefetched when the reader opens the panel, not +when a pathway loads. + +**Why**: The panel is hidden with `display`, never destroyed, so an ungated +prefetch fired **thirteen requests to third-party PSICQUIC servers on every +pathway anyone opened**. Measured on R-HSA-1368108: a reader who never opens the +panel made **14** interactor requests; now **1**. + +It also starved the same-origin connection pool — browsers allow about six +connections per host — and broke an unrelated test, the Molecules tab, which +timed out waiting for data behind those requests. That failure is what made the +cost visible. + +Opening the panel is the moment the counts are wanted, so nothing is lost. + +--- + +## D10. The control is called "Interactors" + +**Decision**: The header control says "Interactors", with the chosen resource +beneath it. + +**Why**: It said "Overlay" — the category it belongs to, not the thing it turns +on. A reader looking for interactors had no word to look for, and two readers in +a row missed a feature that was working. + +**Rejected for now**: moving it to the diagram toolbar, which is where production +puts it (#200 suggestion 2). That is a layout change, not a label change, and is +still open. + +**Rejected**: matching production's red badge (#200 suggestion 4). Disease badges +are already red (`#BA1A1A`) and ordinary interactor badges purple (`#68297C`); +making ordinary badges red would make the two indistinguishable. That needs a +decision about which distinction matters more, not a colour swap. + +--- + +## D11. The header controls stay divs + +**Decision**: They keep their markup and gain `role`, `tabindex`, key handling, +labels and `aria-controls`, rather than becoming `