Skip to content

Open-issue triage & execution plan (living doc) #656

Open
Open
Open-issue triage & execution plan (living doc)#656
Labels
area:infraArea: infrastructure/buildeffort:sEffort: <= 1 daypriority:p3Priority: low / polishstatus:in-progressStatus: in progresstype:choreType: infra/dev task, no user-facing change

Description

@skearnes
skearnes
opened on May 30, 2026

ORD-App Open-Issue Triage & Execution Plan

This plan lives in two synced places: GitHub issue #656 (canonical, shareable) and a local working copy at ISSUE_TRIAGE_PLAN.md (git-excluded, not committed). Edit both in place to keep them current rather than re-filing.

Snapshot: ~110 open issues as of 2026-05-30 (started at 115; 5 closed this session — see below). This is a working plan, not a commitment — re-label and re-prioritize as you go. The mapping table at the end assigns every open issue to a cluster and wave so nothing is dropped.

⏱ Status update — 2026-06-20: major bug clusters cleared

Open issues down to 62 (~61 excluding the plan #656 + epics #662/#671/#681). Most of Waves 1–4's tractable bugs were implemented, runtime-verified against a live no-auth stack (Playwright screenshots + a seeded second user for multi-user/permission scenarios), and merged:

What remains now needs maintainer input, not more solo fixes: design specs (#605/#555/#209/#604/#590/#286/#619), BE-foundational scoping (#634/#474/#261/#259/#258/#43 — repo admin is the BE owner), and deeper bugs (#613/#369/#289). The clean verify-and-close + low-risk-additive queue is exhausted.

Status — labeling scheme applied (2026-05-30): the §2 labels have been created and applied to all open issues (additive; existing semantic labels kept). effort: and priority: for unprioritized issues are heuristic first-pass values — adjust freely. An earlier snapshot of this plan undercounted because the issue listing was capped at 100; the older issues (272, 261, 259, 258, 237, 234, 233, 215, 209, 203, 189, 65, 62, 43) are now folded into the clusters below.

Closed this session (2026-05-30) — verified obsolete/already-fixed (open count now ~110):

Merged this session (all squash-merged to main, 5/5 Greptile, green CI, branch up to date; review-rule the only admin override):

Greptile policy enforced on merge: 5/5 confidence required; #660 and #667 started at 4/5, the flagged items were addressed (→ 5/5) before merging.

⚠️ Deployed image is stale — built 2026-05-11. The currently deployed app image was built on 2026-05-11, so the live site lags main by weeks. Bugs observed only on the deployed site (not reproducible on main/the local no-auth stack) may already be fixed in code and are likely stale-deployment artifacts — confirm against main before filing/coding. This is the most probable explanation for the #554/RpM "Type" placeholder no longer existing in the codebase. A redeploy may close such issues outright.

Maintainer decisions resolved (2026-06-16) — these unblock the items below:

Followups & clarifications (blocked, not codeable as-is):

Testing infrastructure (epic #662 — see §9). Core COMPLETE (all merged):

Project board (2026-05-30): Project #12 — "ORD-App Issue Triage" (public). All 113 open issues added, each tagged with a single-select Wave field (Wave 0–5 / Backend track / Triage-to-close) matching the coverage map. Group the board view by Wave to get wave columns; group/filter by the priority:/status: labels for the other axes.

  • Org-level, not repo-level: GitHub Projects v2 only exist under a user/org — repo-level boards were the deprecated "classic" Projects. The board is not linked to the repo: linking bulk-imports every repo issue (incl. closed ones), which would pollute the curated set, so it's left unlinked and accessed via the org URL. To link anyway, first disable the project's auto-add workflow in the UI.

1. Why the backlog is hard to action today

  • Labels are inconsistent. Some issues carry Front-end/Back-end, some carry priority (high/medium/low), many carry neither. clarify and Full scope are used as catch-alls. You can't slice the backlog reliably.
  • Many issues share a root cause but are filed separately (e.g. five "needs page refresh" bugs, three "Paste Chunk drops/keeps data" bugs, four "text doesn't truncate" bugs). Fixing them one-by-one is wasteful; fixing them by root cause is a handful of PRs.
  • Epics are mixed in with bugs. [ORD-*] trackers and Initiative/Feature issues aren't "fixable" — they decompose into child work.
  • A few are already fixed or are upstream/3rd-party and just need closing or a documented workaround.

The plan below fixes all three problems: a labeling scheme, root-cause clustering, and a wave-based execution order.

2. Labeling scheme (apply first — ~1 hour, unblocks everything)

Give every issue exactly one value on each axis. Keep existing semantic labels (good first issue, 3rd party, etc.).

Axis Values
area: fe, be, infra, design
type: bug, enhancement, epic, chore
effort: xs (≤½ day), s (≤1 day), m (1–3 days), l (>3 days / needs design)
priority: p0 (blocks usage / data loss), p1 (high), p2 (medium), p3 (low / polish)
status: needs-triage, needs-clarify, ready, blocked, in-progress

Suggested GitHub automation: a saved view per area:, and a project board with columns = waves below.

3. Triage rules (how to decide, fast)

  1. Is it an epic/tracker? → label type:epic, link children, never close until children done. (Cluster A.)
  2. Does it need the backend? → area:be + status:blocked if FE can't proceed. (Cluster B.)
  3. Is the expected behavior undefined? (clarify issues) → status:needs-clarify, write the acceptance criteria with the PM before any code. (Don't guess on these.)
  4. Does it share a root cause with siblings? → tag with the cluster name; fix the cluster in one PR, close the siblings together.
  5. Everything else → estimate effort: and priority:, drop into a wave.

Definition of done for FE bugs: because almost every issue is visual, a PR must include before/after screenshots (or a short clip) for each issue it closes, plus the tsc/lint/test gates already enforced by CI.

4. Root-cause clusters

Issues grouped by where the fix actually lives. Fix the cluster, not the ticket.

A. Epics / trackers / umbrellas — decompose, don't "fix"

626, 627, 628, 320, 353, 394, 455, 542

Action: convert each into a checklist of child issues, then close when children close. 455 and 542 are umbrella "inconsistencies/enhancements" issues — split into concrete child tickets or close as duplicates of the clusters below.

B. Backend-owned / FE-blocked

634, 569, 474, 322, 597, 631, 261, 259, 258, 234, 43

Action: hand to BE owner. 597 (validation hidden by DOI error) and 631 (Auth0 refresh, p0, high) likely need both FE and BE — pair them. 322 duplicates epic 320. Foundational/template BE work: 43 (prepare API + tables), 258/259 (save-reaction-as-template + templates page, back 65/525), 261 (add reactions to dataset after enumeration, backs 474), 234 (ORCID/email manual setup, see Cluster T).

C. Validation & invalid-reaction surfacing

62 (epic), 369, 591, 466, 461, 457, 446, 622 (+597 from B)

Shared concern: how invalid reactions are computed, labeled, filtered, and displayed. 62 [ORD-6.7] Validations is the parent epic — decompose it across the children here. Design the data flow once (where validation state is stored, how it's surfaced in the table/labels/filters), then implement 466/461/457 together. 591 and 446 are "define the behavior" prerequisites — resolve them first.

D. Permissions / role-change reactivity

617, 615, 614, 610, 616, 399, 288

Shared root cause: the UI doesn't react to role/permission changes without a refresh, and gating (isAdmin/isEditor/isViewer) is scattered. Centralize a permission selector + make role state live (poll or push), then 617/615/614/610 mostly fall out. 616 (notify on access loss) and 288/399 (group-role display/filtering) are smaller adjacent fixes.

E. Stale data / needs-refresh (live updates)

611, 586, 584, 431, 624

Shared root cause: lists/views aren't invalidated/refetched after mutations (create-via-enumeration, delete-last-on-page, deleted-by-other-user, reaction-remove). Audit the data-fetching layer's cache-invalidation/refetch on these mutations; one consistent strategy closes most. 624 (automation-code vars dropped on refresh) is a persistence bug — adjacent but separate.

F. Paste Chunk behavior

601, 592, 589

Almost certainly one code path. 601 (duplicate automation codes), 592 (measurements/record-mods retained when excluded), 589 (structure not deleted) — fix the paste/merge logic and its field-inclusion handling together.

G. Text formatting / truncation — good first issue cluster

189 (parent), 355, 356, 357, 358, 511

189 [General Improvement] Characters truncation… is the parent story the four Characters truncation issues reference. Build one reusable "truncate-with-tooltip-on-hover" treatment, apply it to Group name / Dataset name+description / user name+email / Reaction ID across tables, sidebars, and headers. 511 (character limits) is the input-side counterpart. One cohesive PR (or a short series sharing the component).

H. Required-field markers & modal validation — partially DONE

345, 342, 454 → in flight in PR #654; remaining modal-validation polish folds into Wave 2.

I. Quick labels / typos / wording — low effort

549, 554, 338, 639(done), 552 ✅closed, 553 ✅closed

552/553 were already fixed in code (commit c7b11e5 / #533) and have been closed. 639 done in PR #655. 549 (relabel "Isolated Product Characteristics") and 554 (placeholder "Type") need a wording decision — status:needs-clarify. 338 (wrong "no user" error when user is already a member) needs the shared thunk to preserve HTTP status (404 vs 409) + a clearer BE 409 message — small but spans FE/BE.

J. Sidebar / entity-form data display

623, 608, 609, 598, 599, 555, 605, 604, 603, 590, 487, 483, 286, 513, 612, 476, 436

Largest cluster; not a single root cause — triage individually but batch by entity:

  • Conditions: 623 (electrochem voltage fields), 590 (preview)
  • Outcomes/Products: 608 (yield missing), 598 (preview no yield/conversion), 599 (ordering by reaction time), 555 (extra vertical space), 487 (limiting-reactant label)
  • Measurements: 609, 604, 603, 483, 286, 513, 613 (file name should be numbered / match Observation name)
  • Misc fields: 612 (chmo_ID help), 476 (URL feature not clickable), 436 (amount unit display), 605 (preview misalignment)

K. Enumeration UX

510, 501, 515, 567 (+614, 611 cross-listed)

CSV validation warnings (510), keep-sidebar-open during matching (501), set-variables on empty structure (515), cancel-confirmation (567).

L. File-upload limits

566, 543

Both are "uploads exceed the limit without an error." Single fix: enforce per-file and cumulative size limits with a clear notification.

M. Loaders / notifications / feedback

382, 312, 209, 414, 314, 293

Loading indicators (382 download, 312 dataset info, 209 — duplicate of 312, adds Group views), delete notifications (414), remove-button hover (314), unhandled console errors (293). Merge 209 ↔ 312.

N. Look up / search

465, 551, 527

Search by InChI/SMILES (465, also touches BE), cursor focus on add-via-lookup (551, xs), structure preview after lookup (527).

O. Dates

619, 544, 289

Timezone-aware last-modified (619), invalid-date warning (544), date-picker edge cases (289).

P. Reaction ID handling

537 (+358 cross-listed)

Trim leading/trailing whitespace when opening by Reaction ID (537, xs).

Q. Layout / responsiveness

292, 408

Group-management sidebar misalignment on resize (292, good first issue), horizontal scrollbar with large reaction data (408).

R. 3rd-party / environment — likely won't-fix / document workaround

587, 449, 410

Chrome copied-image label clipping (587), Ketcher Escape handling (449), Global Protect VPN blocks app load (410). Investigate feasibility; if upstream, document a workaround and close as wontfix.

S. Larger features / product decisions

632, 525, 287, 580, 523, 550, 496, 621, 237, 203, 65

System config for conditions/summary (632), reaction-parts-as-templates (525), share dataset (287, depends on BE 634), replace mocked data (580), printable preview list (523), save-and-close (550), template 404 (496), Variable-link page error (621), duplicate reaction (237), Contribute landing page (203), create-reaction-from-saved-template (65, depends on BE 258/259). ORCID refresh (396) moved to Cluster T.

T. Auth / ORCID / user profile

272, 233, 234, 215, 396 (+631 from B)

ORCID/email/name handling. 233/234/215 are the [ORD-11.10] trio (general / BE / UX) for manual ORCID-email-name setup — treat as one feature, three facets. 272 (ORCID data not available after trusting the app) and 396 (refresh ORCID data without relogin) are related bugs/enhancements. 631 (token refresh) is the auth-infra dependency.

5. Execution waves (recommended order)

Status (2026-05-30): 106 open issues remain. Most recent effort went to the cross-cutting initiatives — the testing-infrastructure epic (§9), the SonarCloud gate (§10), and the dev-tooling modernization (§11) are ✅ done — plus Wave 0 and a handful of Wave 1 quick wins. The product backlog itself (Waves 2–5) is still largely untouched and is the main remaining work.

Wave 0 — Close the books. ✅ DONE.
Merged #654 (345/342/454 required asterisks) and #655 (639 typo). #552/#553 verified & closed. Labeling scheme applied.

Wave 1 — Quick confident wins. 🟡 Partially done.
Shipped: #657 (Cluster G — full-text hover on truncated names/IDs), #658 (#537, Cluster P — trim Reaction ID before search), #660 (#567, Cluster K — confirm before cancelling Enumeration), #659 (distinct error for existing group member), #661 (sort grouped roles alphabetically).
Still open: Cluster I wording (338, and 549/554 pending clarification), the rest of the Cluster G truncation set (355–358/511), and the xs one-offs 551/612/436/487.

Wave 2 — Themed bug batches (root-cause PRs). ⬜ not started.
F (Paste Chunk 601/592/589) → E (refresh/staleness 611/586/584/431) → D (permissions reactivity 617/615/614/610/616). These are the p1/p2 correctness bugs and each batch is one focused PR.

Wave 3 — Validation surfacing (Cluster C). ⬜ not started.
Resolve 591/446 (define behavior) first, then implement 466/461/457/622/369 together. Coordinate 597 with BE.

Wave 4 — Entity-form / sidebar display (Cluster J), Enumeration (K), Uploads (L), Loaders (M), Dates (O), Lookup (N), Layout (Q). ⬜ not started.
Mostly independent s/m items — parallelizable across contributors once labeled.

Wave 5 — Features & epic decomposition (Clusters S, A). ⬜ not started.
Break epics 626/627/628/320 into children; schedule features 632/525/287/580/396/523/550/496/621 by product priority.

Backend track (parallel, BE owner): Cluster B (634/569/474/322/597/631/261/259/258/234/43) — start 631 (auth, p0) and 597 immediately; 43/258/259/261 are foundational and unblock template + enumeration features.

Triage-to-close track: Cluster R (587/449/410) — investigate, document, close.

6. Coverage map (every open issue → primary cluster → wave)

Cluster Issues Wave
A Epics/umbrellas 626, 627, 628, 320, 353, 394, 455, 542 5
B Backend 634, 569, 474, 322, 597, 631, 261, 259, 258, 234, 43 BE track
C Validation 62, 369, 591, 466, 461, 457, 446, 622 3
D Permissions reactivity 617, 615, 614, 610, 616, 399, 288 2
E Stale/refresh 611, 586, 584, 431, 624 2
F Paste Chunk 601, 592, 589 2
G Text truncation 189, 355, 356, 357, 358, 511 1
H Required-field markers 345, 342, 454 0 (PR #654)
I Labels/typos/wording 552, 553, 549, 554, 338, 639 0–1
J Sidebar/entity display 623, 608, 609, 598, 599, 555, 605, 604, 603, 590, 487, 483, 286, 513, 612, 476, 436, 613 1/4
K Enumeration UX 510, 501, 515, 567 4
L Upload limits 566, 543 4
M Loaders/notifications 382, 312, 209, 414, 314, 293 4
N Look up/search 465, 551, 527 1/4
O Dates 619, 544, 289 4
P Reaction ID handling 537 1
Q Layout/responsiveness 292, 408 4
R 3rd-party/environment 587, 449, 410 triage-to-close
S Larger features 632, 525, 287, 580, 523, 550, 496, 621, 237, 203, 65 5
T Auth/ORCID/profile 272, 233, 234, 215, 396 1/5

Cross-listed (appear in a cluster's prose but counted once above): 597 (B+C), 614/611 (D/E + K), 358 (G + P), 286 (J + S), 631 (B+T), 234 (B+T), 258/259 (B↔S templates), 209↔312 and 320↔322 and 543↔566 and 233/234/215 are likely duplicates/facets to merge.

✅ Closed by this effort (2026-05-30): Wave 0 — 552, 553, 639, 345/342/454 (#654/#655). Wave 1 — 537 (#658), 567 (#660), the truncation hover win (#657), plus #659/#661. Testing-infra epics 663/664/665 (#666/#667/#668/#669). 106 issues remain open — the table above is still essentially the full product backlog (Waves 2–5 untouched).

🔄 §9.4 unit-test backfill (2026-06-01..02): Merged PRs #679, #686–#693, #695, #697–#702 (+ #694 Codecov/coverage reporting, #696 hook-deps fixes). Backfilled converters / transforms / selectors / reducers / registries / utils / hooks, then the component layer via three render helpers (#703 adds renderInReactionView). Vitest suite ~220 → 411 passing; fixed a real VesselAttachments converter bug (#699). Remaining tail: async thunks + ~50 deep ReactionView/entity-node wrappers needing bespoke fixtures — see §9.4. No product issues closed by this batch; it's coverage groundwork.

🔄 §9.4 backfill, continued (2026-06-03): PRs #707–#722 took UI coverage 48.4% → 52.0% lines / ~80% branch, 554 tests (173 files). Filled the remaining high-value logic — web workers + both worker middlewares, useAuth, the reactions reducer (57→90%) + converter orchestrators (49→100%), reactions.utils (45→97%), and assorted hook factories/pure utils — plus a latent previews-worker null-guard fix (#712) and a dead-code removal (#722). Remaining: async thunks (axios harness), intricate proto leaf converters, the yup reactionEntityToValidation, and bespoke-fixture view components. Still coverage groundwork; no product issues closed.

7. Immediate next actions

✅ done (2026-05-30): verified/closed #552/#553; merged #654/#655; applied labels; shipped the Wave 1 quick wins #657/#658/#659/#660/#661; stood up the full testing-infrastructure epic (#666/#667/#668/#669); cleared the SonarCloud gate (§10) and modernized dev tooling (§11).

Remaining, in priority order:

  1. Maintainer decisions that unblock work: resolve the needs-clarify issues (549, 554, 591, 446) and decide the ty tail items in Adopt ty (Astral type checker) for ord_app #681 (transient validation design, None-safety, SQLAlchemy contracts).
  2. Kick off the BE track on 631 (Auth0 refresh, p0) and 597 (validation hidden by DOI error).
  3. Finish Wave 1: the rest of the Cluster G truncation set (355–358/511) and the xs one-offs (551/612/436/487).
  4. Then Waves 2→3 — the themed root-cause bug batches (F → E → D, then Cluster C validation), each one focused PR.
  5. Grow test coverage alongside each fix (§9.4), and pair the logic-affecting Sonar tail (§10) with that coverage.

8. Notes & flags

9. Testing & verification infrastructure (recommended initiative)

Why this is high-leverage. Today the only automated UI check is one placeholder vitest spec, the UI is Auth0-gated, and there's no browser automation — so the bulk of the open bugs (visual/behavioral) can only be caught by manual QA, and neither a contributor nor an agent can self-verify a UI change. Building this out turns dozens of "needs a visual pass" issues into testable ones and is a prerequisite for confidently clearing Waves 2–4.

Everything here must be reproducible and CI-runnable — no machine-specific setup. The repo already does this for the Python side: uv sync --frozen (pyproject.toml + uv.lock) for deps, and the test_python CI job provisions Postgres with ikalnytskyi/action-setup-postgres@v7, with the backend reading PG_DSN / PG_ALEMBIC_DSN / PG_TEST_DSN. The testing build-out follows the same model (lockfiles + provisioned services), so it runs identically on any contributor's machine and in CI. Grounding facts (verified 2026-05-30): vitest already runs in CI (test_ui → npm run test); stack is React 19 / Vite 6 / Vitest 3.

9.1 Browser automation / E2E + visual — Recommend: Playwright

Add @playwright/test, playwright.config.ts, an e2e/ dir, and npm run test:e2e. Gives headless Chromium (+ optional Firefox/WebKit), screenshots, the trace viewer, and toHaveScreenshot() visual-regression baselines — and it's what lets an agent render + screenshot a page to verify UI.

  • Over Cypress: faster, true multi-browser, first-class CI + trace debugging, built-in visual diffing.
  • Over Storybook + Chromatic: lighter and free (Chromatic visual-regression is paid SaaS); add Storybook later if we want isolated component states.
  • Add a test_e2e CI job (build → preview/webServer → playwright), non-blocking at first, promote to required once stable.
  • Revisit trigger: if per-component isolation matters, add @playwright/experimental-ct-react or Storybook.

9.2 Non-auth path for local/test — Recommend: a dev-only auth-bypass flag

Add e.g. VITE_E2E_NO_AUTH that — only in dev/test builds — makes useAuth resolve to a mock authenticated user and injects a dev token into the axios client, paired with the backend's existing ORD_APP_TESTING=TRUE (confirm it accepts/relaxes the test token).

  • Hard guard: must be compile-time impossible in prod — gate on import.meta.env.DEV and the explicit flag, plus a test asserting prod bundles never enable it. This is security-sensitive; the bypass must never ship.
  • This is the key enabler — it lets Playwright (and plain local dev) reach the authenticated screens without an Auth0 round-trip. The alternative (Playwright storageState from a real Auth0 login) is brittle and needs shared creds — not the primary path.

9.3 Full local stack — reproducible, no machine-specific deps

  • Python deps: uv sync --frozen (pyproject.toml + uv.lock). No global/conda installs.
  • Postgres: provision reproducibly, never from a personal environment —
    • Local: docker compose up -d (the repo's docker-compose.yml already provides Postgres + backend), or any local Postgres.
    • CI: the E2E job mirrors test_python's ikalnytskyi/action-setup-postgres@v7.
    • Point the backend at it via PG_DSN / PG_ALEMBIC_DSN / PG_TEST_DSN (+ ORD_APP_TESTING=TRUE).
  • Backend: uv run fastapi dev ord_app/service_api/main.py with the PG_* env set (serves :8000).
  • Frontend: cd ui && VITE_E2E_NO_AUTH=TRUE npm run dev (after 9.2) → :5173 against the test backend, no login.
  • E2E: Playwright webServer boots both (or runs against the pair); seed data via the API.
  • Add a make dev / scripts/dev.sh (docker-compose Postgres + uv run backend + UI) so it's one command, identical locally and in CI.

9.4 Unit tests — Recommend: test-as-we-go + targeted backfill PRs

✅ Harness done: vitest + happy-dom + @testing-library/react + @vitest/coverage-v8 (pinned to the vitest major). vitest is scoped to src/** so it doesn't pick up the Playwright e2e/ specs.

9.5 Suggested sequencing

  1. ✅ PR A — test harness: happy-dom + testing-library + coverage config + first real unit tests. Done.
  2. ✅ PR B — no-auth dev flag (9.2): VITE_E2E_NO_AUTH frontend + APP_ENV=localhost backend allowlist, hard-gated off in prod, with guard tests (Add dev-only Auth0 bypass flag (frontend half of no-auth path) #667, Add backend e2e no-auth mode (backend half of no-auth path) #668).
  3. ✅ PR C — Playwright (9.1): smoke E2E (no-auth → datasets) + test_e2e CI job (Postgres service, SHA-pinned actions) + the scripts/dev-e2e.sh dev script (9.3). Validated the prod auth guard end-to-end.
  4. In progress: converter/reducer/selector backfill PRs (§9.4). Next: convert each "needs visual verification" issue (Move cursor immediately to entry field when Add Component via Look up Name is used #551, Yield value missing in product sidebar  #608, Misaligned preview in input/outcome list/sidebar requires adjustment #605, Bottom scroll bar issue when viewing large Reaction data. #408, the truncation visual sub-parts, The "Limiting Reactant" label is not appearing in the Reaction pane. #487) into an E2E/visual test as it's fixed.

Tracked under the "Testing infrastructure" epic (#662).

9.6 Dependency hygiene — make the build less brittle

  • Lock and pin everything. Python: pyproject.toml + uv.lock via uv sync --frozen. JS: package.json + package-lock.json via npm ci. Avoid floating/* ranges for tooling — a @vitest/coverage-v8@* pulled v4 against vitest v3 in PR A (Set up UI unit-test harness (Testing Library + coverage) #666); pin to the matching major.
  • No machine-specific dependencies in any test/run path. Postgres, browsers (Playwright installs its own), and all tooling come from lockfiles + docker/CI services, so a fresh checkout runs the full suite identically on any machine and in CI — never from a contributor's global/conda environment.
  • Prefer lighter, well-maintained deps when a transitive conflict appears (e.g. PR A switched jsdom→happy-dom to dodge a transitive ESM/CJS break and shrink the tree).
  • CI is the source of truth. If it doesn't run green in CI from a clean checkout, it isn't done.

10. SonarCloud debt cleanup (initiative)

Goal: get the SonarCloud quality gate green on main and keep it green — addressing all open issues, including pre-existing ones from before this plan. Tracked in #671.

Process note (learned the hard way): SonarCloud Code Analysis is part of CI. A PR is not green unless its SonarCloud gate passes; never merge over a red Sonar check. Each cleanup PR must itself pass the PR gate (introduce no new issues).

Current state (2026-05-30): ✅ quality gate GREEN on main (all conditions OK). Open issues 162 → 93. The remaining ~93 are documented won't-fix or logic-affecting (see below).

Batches (one or more PRs each):

  1. Bugs / reliability (4) — ✅ DONE (Fix SonarCloud reliability bugs (task GC + async file I/O) #672): main.py background-task lifecycle (keep + cancel the task ref); test file-I/O moved out of async def via sync conftest helpers. Cleared new_reliability_rating to A.
  2. Security hotspots — ✅ GitHub Actions SHA-pinning (Pin GitHub Actions to commit SHAs (SonarCloud hotspot) #670). The ui/index.html gtag + Google-Fonts resource-integrity hotspots were reviewed "safe" by the maintainer in the SonarCloud dashboard (SRI on rotating CDN scripts is impractical) → new_security_hotspots_reviewed now OK.
  3. Code smells — ✅ cleared the high-value mechanical rules: Number.parseInt/parseFloat (Sonar: prefer Number.parseInt/parseFloat #673), python:S1481 unused locals→_/removed (Sonar: remove unused unpacked locals (S1481) #674), unnecessary type assertions S4325 (Sonar: remove unnecessary type assertions (S4325) #675, 5 real + 4 documented false positives), globalThis over window (Sonar: prefer globalThis over window (S7764) #676 .ts, Sonar: globalThis over window in gtag snippet (S7764) #678 index.html), Set.has over .includes (Sonar: use Set.has over Array.includes (S7776) #677).

Won't-fix (Sonar false positives for this codebase), documented in #671:

  • typescript:S7772 (node:buffer over buffer, 10) — Vite browser app uses the buffer polyfill; node:buffer breaks the build.
  • python:S7503 (remove async, 21) — async test fns must stay async def to consume async fixtures under pytest-asyncio.

Remaining (logic-affecting — defer behind test coverage): S7735 negated-condition branch swaps (11), S2486 empty-catch handling (5), S6582 &&→?. (3), plus S1192 string constants (some in generated alembic migrations) and assorted singletons. These change control flow; pair with the unit-test backfill (§9.4) rather than fixing blind.

Ordering: ✅ batches 1–3 done and gate is green. Remaining tail is won't-fix or logic-affecting; revisit after coverage grows.

11. Dev-tooling modernization (mirror ord-schema)

Goal: align ord-app's tooling with the sibling ord-schema repo's Astral stack (uv / ruff / ty / addlicense via pre-commit).

  • ✅ pre-commit + addlicense (Adopt pre-commit with addlicense; drop custom license template #680): replaced the year-locked license-check-and-add template (ui/FILE_LICENSE, license-checker-config.json) and husky/lint-staged with a .pre-commit-config.yaml running google/addlicense (year-agnostic — new files use the current year), Ruff (ruff + ruff-format, scoped to ord_app/), and the UI Prettier/ESLint/Stylelint via local hooks. One-off ruff format over ord_app/ + a ruff format --check CI step. Devs run uv run pre-commit install.
  • ✅ ruff config consolidation (Consolidate ruff config into pyproject.toml; remove ruff.toml #682): folded the root ruff.toml (which shadowed [tool.ruff]) into pyproject.toml as the single source of truth.
  • ty adoption (Adopt ty (Astral type checker) for ord_app #681) — in progress: load_message made generic (Make load_message generic over the proto message type (ty) #683), schema constr()→Annotated[StringConstraints] (Use Annotated[str, StringConstraints] over constr() in schemas (ty) #684), run_in_threadpool casts (Cast run_in_threadpool(load_message) results to Reaction (ty) #685). 60 → 22 diagnostics. The remaining 22 need a maintainer decision (triaged in Adopt ty (Astral type checker) for ord_app #681): the transient ReactionModel.validation attribute (SQLAlchemy non-mapped design), None-safety guards (possible latent bugs), SQLAlchemy return-type/method-override contracts, pydantic model construction, and a few third-party stub # ty: ignores. Once at zero, a final PR adds the ty hook + dev dep + CI step (and decides whether to drop pytype).
  • ✅ ruff ruleset alignment with ord-schema (chore: align ruff ruleset with ord-schema (add W/UP/SIM, full E) #785): broadened ord-app's select from E4/E7/E9, F, I, B to ord-schema's E, F, W, I, B, UP, SIM (full pycodestyle + warnings, pyupgrade, flake8-simplify). Full E/W added zero new violations; the delta was UP/SIM modernizations (Optional/Union→X | None, List/Dict→list/dict, typing.Sequence→collections.abc, isinstance unions, yoda conditions). ignore tailored to ord-app's actual trips (not ord-schema's debt suppressions); tests keep B011/B017.
  • ✅ full annotation + FastAPI rulesets (chore: require full type annotations (ANN) + flake8-fastapi (FAST) #786): turned on all of flake8-annotations (ANN: arguments, *args/**kwargs, return types) and flake8-fastapi (FAST) for ord_app/, annotating every production signature (tests exempt). ANN401 (no bare Any) intentionally left off — Any is the honest type for **kwargs forwarding (HTTPException/SQLAlchemy filters) and pydantic before-validators, so it would only add noqas. The annotations surfaced + fixed real type facts: ReactionModel.is_valid→Mapped[bool | None], GroupModel.role declared as a non-mapped transient attribute (like validation), 204 handlers→-> None, response_model=None where the return isn't a pydantic field, and None-safety guards (extend→404). FAST002 autofixed (Annotated dependencies).
  • Also fixed: a pre-existing flaky boundary in the *_with_character_limitations tests (min_chars=MAX allowed an exactly-MAX value that passes the > MAX check) — now generates strictly over each field's own limit (Fix flaky character-limitation tests (boundary off-by-one) #688).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:infraArea: infrastructure/buildeffort:sEffort: <= 1 daypriority:p3Priority: low / polishstatus:in-progressStatus: in progresstype:choreType: infra/dev task, no user-facing change

    Type

    No type

    Projects

    ORD-App Issue Triage
    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions