Skip to content

Feature/verification api gui - #126

Merged
HarrisonCreates merged 23 commits into
mainfrom
feature/verification-api-gui
Oct 22, 2026
Merged

HarrisonCreates merged 23 commits into
mainfrom
feature/verification-api-gui

Conversation

@HarrisonCreates

Copy link
Copy Markdown
Collaborator

Introduces the Verification API GUI to the main branch along with its corresponding changes

Should remain a draft until the additional verification API endpoints are merged into main.

@NovrusShehaj NovrusShehaj self-assigned this Oct 21, 2026
@NovrusShehaj

Copy link
Copy Markdown
Collaborator

PR #126 Review: Feature/Verification API GUI (Backend & API Contract)

Verdict

Not ready to merge. The UI work is substantial and mostly well wired, but it's coded against a backend contract that doesn't exist yet. It is also incompatible with the backend PR it's waiting on (#136). Once #136 merges, the "Lookup Past Verifications" tab breaks.

Scope note: The PR changes 0 backend files (11 frontend files, +5,084/−9). This review therefore covers what the backend has to support it: every endpoint it calls on origin/main, the pending backend PR #136, and the backend gaps the frontend is working around. Line numbers refer to the PR head (6fd1c88).

Merge-Readiness Status

Gate State
Draft Yes. The description says to keep it as a draft until the backend verification endpoints merge.
Mergeable Conflicting in frontend/app/app.config.ts. It's trivial: both sides add lenis-prevent slot config, so take the union.
backend-ci ❌, but not caused by this PR. All 13 errors are the Keycloak testcontainer returning 404 on /health/ready. main's latest run also fails, on a Temporal dev-server download error. A rerun after rebasing should clear it.
frontend-ci / smoke ✅
Reviews None
Backend dependency (#136) Open, behind main, backend-ci failing (same flake), one "commented" review.

Endpoint Contract Check

Frontend call Exists on main? Notes
POST /compatibility/check ✅ omex_id is md5(file_bytes), the same hash as OmexFile.file_hash_md5, so hash linking can work.
POST /verify/omex ✅ The omex_hash query param the frontend sends is ignored (neither main nor #136 accepts it).
POST /verify/runs ✅ omex_hash is ignored on main; it only exists in #136.
GET /verify/{id} ✅ Status enums match exactly.
GET /verification_ids ✅ main returns only verification_ids; records comes from #136 with a different shape (finding 1).
POST /simulations/runs ✅ Request body is valid (perPage: 100 is the max).
{legacy_api_url}/runs/{id} and /download Bypasses the backend The backend already proxies these at GET /runs/{id} and GET /runs/{id}/download, with rate limits.
https://simdata.api.biosimulations.org/... Hardcoded Not in runtime config, so it's wrong for the RKE and local overlays.

Blockers

1. Contract mismatch with #136 breaks the Lookup tab

2. The backend half isn't merged

On main today, every hash feature is silently inert:

  • The "this archive has N past verifications" banner.
  • The hash badges on cards.
  • The ?omex_hash= deep link.
  • The omex_hash linkage on /verify/runs.

The page doesn't crash, because it falls back to verification_ids. But it ships features that do nothing.

High-Severity Findings

3. A stale report can be shown under a different workflow ID

Location: verify-model.vue:815–851, 890–901

  • openHistoricalModal, loadWorkflowById, and the route.query.workflow_id watcher change activeWorkflowId but never reset workflowOutput or precheckResult.
  • VerificationStatus then seeds itself from the old modelValue. If the new fetch fails (401, 403, 404 "no longer retained", 503), the previous verification's results stay on screen labelled with the new ID.
  • There's also no abort or ID check in fetchStatus, so a late response for the old ID can overwrite the new one.

4. Polling never stops on errors and has no in-flight guard

Location: VerificationStatus.vue:97–120

  • The catch sets a message but leaves the 5 s interval running, so a permanent 401/403/404 polls forever while the page is open.
  • Each poll costs the backend a Temporal describe plus a query, each with a 60 s rpc_timeout (api/main.py:655).
  • GET /verify/{id} has no rate limit, and with feat(verify): link OMEX hash to verification runs and return run metadata #136 every poll also writes to Mongo.
  • When Temporal is slow, requests overlap and pile up.
  • Fix: Stop on 4xx, back off on 5xx, and skip a tick while a request is in flight.

5. include_outputs: true is the GUI default

Location: verify-model.vue:63

The backend default is False.

  • Full trajectories travel through the Temporal activity result and the workflow query (biosim_verify/activities.py:147), with no size guard.
  • Temporal's default blob limit is about 2 MB. Rough estimate: 100 variables × 1,000 timepoints × 5 simulators × ~20 bytes per JSON float ≈ 10 MB.
  • The stats activity retries up to 100 times. Moderately sized models will likely fail outright. I haven't reproduced this in prod, but nothing in the code prevents it.
  • Fix: Default to false, or have the backend move outputs to storage.

6. URL and platform-run submits re-download the archive in the browser

Location: verify-model.vue:485–495

  • The compatibility check downloads server-side, with SSRF guards, so step 1 passes.
  • Submit then calls fetch(url) from the browser, which depends on CORS and will fail for most third-party hosts.
  • Platform runs use legacy_api_url directly instead of the backend proxy.
  • Backend fix: Let /verify/omex accept the cached omex_id, the same way POST /simulations/run already does via get_omex_file_for_caller (simulations/router.py:222). The frontend already sends omex_hash, which suggests that's what the author expected.

Medium-Severity Findings

7. The client-side precheck duplicates and bypasses the backend

Location: useVerificationAnalytics.ts — precheckRunIds

  • It calls the legacy API and the hardcoded simdata host from the browser. If CORS blocks those, every run is excluded and runs-verification is blocked entirely.
  • The backend already returns 400 when runs share no datasets.
  • The precheck exists because RunsVerifyWorkflow fails the whole job if any run isn't SUCCEEDED (runs_verify_workflow.py:54–67). A backend "skip non-succeeded runs" option, or a precheck endpoint, would be cleaner.
  • It also reads sedmlDataSetIds, while the backend compares on sedmlDataSetLabels.

8. Historical-run discovery is a heuristic

Location: Lines 336–354

  • It takes the newest 100 platform runs (from everyone) and substring-matches the filename stem, so model.omex matches any run named "model".
  • The underlying gap is in the backend: SimulationRunRecord doesn't store the OMEX hash, even though omex_file is available at submit time.
  • Adding the hash plus a filter would make this exact.

9. Internal run IDs leak into run-ID calls

Locations: PlatformRunPickerModal.vue:80–82, verify-model.vue:361,382

  • r.biosimulationsRunId || r.id falls back to the internal UUID.
  • simulations/models.py:137–141 explicitly says not to do that.
  • Nothing gates runs by status either, so CREATED and FAILED runs can be picked.

10. The server-side omex_hash filter goes unused

11. Failed simulators are invisible outside the submitting tab

  • The backend only sets hdf5_file on SUCCEEDED runs, so failed runs are dropped from sims_run_info.
  • The frontend rebuilds the "DROPPED / FAILED" rows from sessionStorage, which permalinks and other devices don't have.
  • This also makes the SUCCEEDED filter (line 954) and the third branch of excludedSimulatorsList (lines 1053–1073) dead code.
  • Backend fix: Return the requested simulators with per-simulator status and error in VerifyWorkflowOutput.

12. Auth timing (likely; worth verifying)

  • onMounted fetches /verification_ids before Auth0 may have restored the session.
  • The first page is then anonymous, the user's own verifications are missing, and nothing refetches when auth state changes.
  • The backend docs say to restart pagination when identity changes.
  • Sharing a permalink to an owned verification gives others a raw 401 or 403, and with finding 4 the page keeps polling it.

Low-Severity Findings

  • Score fallbacks are inverted. score >= 0.99 / >= 0.999 is treated as agreement, but the backend rule is is_close = score < 1, so a low score means agreement. These branches are dead today because the backend always sends is_close.
  • Brittle type detection. Verification type is inferred from the default omex- / runs- ID prefixes. The ledger already stores verify_type; expose it in feat(verify): link OMEX hash to verification runs and return run metadata #136's VerificationRun.
  • Unnecessary preflights. The Cache-Control/Pragma headers and the _t param add a CORS preflight to every anonymous poll for no benefit.
  • Untyped responses. res: any is used for /simulations/runs. Typing it (or generating types from openapi_3_1_0_generated.yaml) would have caught finding 1.
  • Scope creep. ValidationForm.vue strips dark-mode classes from a shared component used by the validate-* pages.
  • Maintainability. verify-model.vue is 2,488 lines and contains two near-duplicate result dashboards.
  • No tests. useVerificationAnalytics.ts is pure logic and easy to test with node --test, like tests/api-auth.test.mjs.

#136 Issues That Affect This PR

  • Ledger status is only updated when someone calls GET /verify/{id}. A verification nobody polls stays PENDING in the listing forever, which matters as soon as the GUI shows status.
  • It writes to Mongo on every poll, even when the status hasn't changed.

What's Done Well

  • It uses the global $fetch, so platform calls carry the Auth0 token and verifications are owner-scoped.
  • The TypeScript models match the backend Pydantic models: VerifyWorkflowOutput, ComparisonStatistics, CompatibilityResponse, and the status enums.
  • List parameters are sent as repeated query keys, name:version simulator specs match _resolve_requested_simulators, and next_cursor pagination is followed correctly.
  • Heatmap indexing matches the backend: matrix row i is sims_run_info[i], the matrix is full N×N, and the code falls back from [j][i] to [i][j].
  • FastAPI detail messages are surfaced consistently.

Path to Merge

  1. Agree on the feat(verify): link OMEX hash to verification runs and return run metadata #136 response shape, then merge feat(verify): link OMEX hash to verification runs and return run metadata #136. That includes fixing its status-staleness and write-per-poll issues.
  2. Update this PR's models and parser to that single shape, use status/created, and pass omex_hash server-side.
  3. Fix findings 3 and 4 (stale output, polling), and default include_outputs to false.
  4. Backend follow-up: Let /verify/omex accept omex_id. Optionally, report per-simulator failures and store the OMEX hash on run records.
  5. Rebase, resolve app.config.ts, rerun CI, mark ready, and request review.

Final note: Separately, the Keycloak and Temporal testcontainer flakes are failing CI on main too and are worth their own fix. I removed the temporary worktree and branch I created; your working tree is unchanged.

Resolves the frontend/app/app.config.ts conflict as the union of both
sides: main's select/table lenis-prevent slots plus this branch's
overscroll-contain body/container slots on modal, drawer and slideover.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@NovrusShehaj

Copy link
Copy Markdown
Collaborator

PR #126 Follow-up Review (head ec4be26)

Verdict

Still not ready to merge. Since my earlier review (against 6fd1c88), the only new commit is a merge of main. It cleared both merge gates but changed none of this PR's code, so all 12 earlier findings are still open. Re-checking them turned up three more medium-severity bugs. The worst is the "Overall Concordance" figure, which can show a green 100% for a run that compared nothing.

Scope note: Links point at ec4be26. Outside app.config.ts, this PR's changes are byte-identical to 6fd1c88, so the earlier review's line numbers still apply.

What Changed Since the Last Review

Earlier Findings

Findings 1–12 and the low-severity items are all still open. Updates on four of them:

  • 1 (contract mismatch with feat(verify): link OMEX hash to verification runs and return run metadata #136): there's now a concrete target: the flat verifications: VerificationSummary[] proposed on feat(verify): link OMEX hash to verification runs and return run metadata #136.
  • 3 (stale report under a new ID): the selected pair and variable also carry over between reports (selectedI, selectedJ, activeVariable, verify-model.vue#L918-L921). Select pair (3, 4) in a five-solver report, then open a two-solver one, and the variable table comes up empty. checkOmexCompatibility and fetchPastVerificationIds (reset vs. load-more) have the same "last response wins" race. One request counter or AbortController fixes all of them.
  • 9 (internal run IDs): the repo already has SimulationRuns/SimulationRun (models/simulators.ts#L76-L103), and its comment says to use biosimulationsRunId, not id, for the legacy API. Typing the two res: any responses with it would have exposed the || r.id fallback.
  • 12 (auth timing): confirmed in code. The global middleware returns early for public routes without waiting for Auth0 (user.global.ts#L41-L44). The Auth0 plugin reads isAuthenticated at request time without waiting for isLoading (auth0.client.ts#L28). Awaiting until(auth0.isLoading).toBe(false) in the plugin's onRequest fixes every page at once, using the pattern the middleware already uses.

New Findings

A. The concordance figure shows 100% when nothing was compared (medium)

  • overallConcordance (verify-model.vue#L1112-L1130) skips pairs with no is_close and returns 100 when none are left.
  • The backend leaves is_close null on every errored pair: missing dataset, label mismatch or shape mismatch (activities.py#L72-L125).
  • If one of two solvers fails, the run still finishes COMPLETED with a 1×1 matrix. The KPI row then reads 100% (green), 1 solver, 0 variables, 0 outliers (green). On another device, the "(1 failed)" hint is missing too (finding 11).
  • The heatmap rounds as well. computePairConcordance uses Math.round (useVerificationAnalytics.ts#L152), and the heatmap colours === 100 as full agreement (VerificationHeatmap.vue#L135). Running the real composable, 199/200 concordant returns 100.
  • Fix: return null when nothing was compared and show "—" with a notice. Report errored pairs instead of skipping them. Colour green only when concordantCount === totalCount.

B. The URL never gets workflow_id after a submit (medium-low, reproduced)

  • After a submit, the page sets activeMode = 'lookup' and then calls router.push to add workflow_id (verify-model.vue#L558-L561). The tab-sync watcher (#L853-L875) runs first and calls router.replace with the not-yet-updated route.query, which cancels the push.
  • Reproduced with the locked vue 3.5.40 / vue-router 5.2.0, with and without an async guard. The URL ends at ?tab=lookup, and the push resolves with "Navigation cancelled … with a new navigation".
  • The same thing happens at #L641-L644 and in "View Past Reports" (#L445-L458), which loses omex_hash.
  • Effect: refreshing during a verification loses the report. With finding 12, the user's own verification may not even be in the list after the refresh.
  • Fix: await router.push(...) before switching tabs (verified to keep workflow_id), or derive the active tab from route.query.tab.

C. The tolerance score is shown backwards (medium-low)

  • The backend passes a variable when score < 1 (activities.py#L192).
  • VerificationTable.vue#L67-L70 renders the score as a percentage under "Tolerance Score", so a pass reads "3.0%" and a failure "450.0%".
  • The fallbacks (useVerificationAnalytics.ts#L141, #L173, #L193) also treat a high score as agreement. Against the real composable, score 0.5 (a pass) counts as disagreeing and 1.5 (a failure) as agreeing.
  • Fix: show the raw ratio labelled "≤ 1 passes" (or "% of tolerance used"), and flip the fallbacks to score < 1.

Lower Severity

  • D. Workflow IDs aren't URL-encoded. The backend accepts anything except / in workflow_id_prefix (api/main.py#L330, #L784), and the listing shows ownerless IDs created by any API client. VerificationStatus.vue#L75 builds /verify/${id} from the raw ID, and so do #L226 and verify-model.vue#L722 for ?workflow_id=${id}. An ID containing #, ? or % hits the wrong path, gets a 404, and then polls forever (finding 4). Use encodeURIComponent and router query objects.
  • E. The CSV export isn't escaped (VerificationTable.vue#L72-L93). Embedded quotes aren't doubled, and leading =, +, - and @ aren't neutralised. Labels come from uploaded SED-ML and ownerless verifications are listed publicly, so a crafted archive can inject spreadsheet formulas for anyone who exports. Also add URL.revokeObjectURL.
  • F. The variable table keeps its page (VerificationTable.vue#L17) when the search, filter or rows change. From page 3, choosing "Failing (2)" or a smaller dataset shows "No Variables Found" even though rows match.
  • G. 422 errors render as raw JSON. Every catch uses err?.data?.detail || err?.message. Clearing a tolerance field sends rel_tol= (v-model.number yields ''), and the 422's list detail is shown verbatim. The backend also accepts any float for the tolerances (api/main.py#L341-L343), so 0 or negative values produce inf/NaN scores. A small apiErrorMessage() helper plus > 0 validation fixes both.
  • H. The residual plot can compare the wrong runs. VerificationPlotViewer.vue#L228-L230 indexes sim_run_data (filtered by dataset) with heatmap positions, so the positions drift when a run lacks that dataset. Key it by run ID instead; runIId/runJId already exist. Likewise, the succeeded filter at verify-model.vue#L954 isn't just dead code. If it ever removed an entry, labels and run IDs would misalign with the matrix. Delete it.
  • Nits:
    • user_description has no input, so every GUI verification is labelled "model-verification".
    • Lenis is prevented four different ways: the app.config.ts slots, per-modal :ui classes, data-lenis-prevent, and the app.vue prevent callback with allowNestedScroll. The app.vue change affects scrolling app-wide and isn't mentioned in the PR description.
    • The heatmap legend shows grey for "Error / Mismatch", but error cells render rose.
    • excludedSimulatorsList writes to submittedSimulatorsMap inside a computed.
    • workflowOutput holds full trajectories in a deep ref, and VerificationStatus deep-watches it. A shallowRef without deep: true is enough.

Corrections to My Earlier Review

  • Simdata host: it is not wrong for the RKE or local overlays. Every overlay's backend uses the same default, simdata_api_base_url = https://simdata.api.biosimulations.org (config.py#L468), and none overrides it. Hardcoding it is only a configurability nit.
  • "FastAPI detail messages are surfaced consistently": this holds for string details only. 422 validation errors carry a list (see G).
  • ValidationForm.vue dark-mode removal: harmless, because the app is pinned to light mode (colorMode.preference: 'light', with no switcher anywhere). It's still unrelated to this PR, and the PR adds a dark: class of its own (PlatformRunPickerModal.vue#L211).

What Checked Out

  • extractTrajectory matches the backend's [num_vars, num_timepoints] row-major layout.
  • The auth plugin only attaches the token to platform-API URLs, so the precheck's calls to the legacy API and simdata don't leak it.
  • Compatible-simulator IDs share the registry namespace with simulator_version.id, so the dropped-simulator matching compares like with like.

Suggested Order

  1. Agree the feat(verify): link OMEX hash to verification runs and return run metadata #136 shape, then point this PR's parser at verifications:
    • use verify_type, status and created;
    • pass omex_hash to the server instead of filtering in the browser;
    • stop sending omex_hash to /verify/omex;
    • delete the optimistic record updates at #L542-L555 and #L625-L638, which the immediate refetch overwrites anyway.
  2. Fix 3, 4, A and B, and default include_outputs to false (#L63).
  3. Add analytics tests next to tests/api-auth.test.mjs. The composable runs under the existing node --experimental-strip-types setup without Nuxt.

Listing: parse the /verification_ids contract proposed on #136
(verifications: VerificationSummary[]) and use its verify_type, status
and created. The omex_hash archive filter is sent to the server instead
of applied in the browser, omex_hash is no longer sent to /verify/omex,
and the optimistic record updates are gone. Backends that only list IDs
still work: type falls back to the default ID prefixes and an archive
filter is reported as unavailable rather than empty.

Reports: switching reports resets per-report state, and late responses
for another workflow, compatibility check or listing page are dropped.
Polling halts on lasting 4xx errors (with a Retry button), backs off on
server, network and retryable 4xx errors, and never overlaps requests.
The Auth0 plugin waits for session restore before sending Platform
requests, so page-load requests carry the user's token.

Concordance: show "—" with a notice when nothing was compared, round
down so 100% means every variable agreed, and colour the summary and
heatmap green only on full agreement. The heatmap legend now matches
the cells.

Navigation: navigate before switching tabs so workflow_id and omex_hash
reach the URL, and close the report when Back removes workflow_id.

include_outputs now defaults to false, matching the backend.

Tests: tests/verification-analytics.test.mjs and
tests/verification-listing.test.mjs run via npm run test:verification
and a frontend-ci step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jcschaff added a commit that referenced this pull request Oct 22, 2026
…derived OMEX hash

Settles the GET /verification_ids contract agreed on PR #136 so the backend and
the #126 GUI converge on one shape:

- `verifications: list[VerificationSummary]` alongside `verification_ids`, same
  rows, same order: workflow_id, verify_type, created (UTC with offset),
  omex_hash, status. Flat, not grouped per page; never carries owner_sub.
  `records`/`run_ids`/`VerificationRun` are gone; `VerificationRecord` stays the
  ledger model (no `VerificationLedgerRecord` rename).
- `status` is a `VerifyWorkflowStatus`, null until GET /verify/{id} first sees
  the workflow finished. Recorded once via a guarded
  `update_one({"status": None})` after authorization, on the recovered paths
  too; nothing is written at insert; a ledger error never fails the GET.
- `omex_hash` is pattern-validated (`^[0-9a-f]{32}$`, else 422) on both routes.
  POST /verify/runs derives it from the platform's cached BiosimulatorWorkflowRun
  records; a caller-supplied hash is stored only for authenticated callers and
  only when nothing is cached, never when cached runs span archives.
- Compound `(omex_hash, created, workflow_id)` index serves filtered pages.
- Tests: 16 exact-dict assertions updated; Mongo-backed filter/scoping/
  pagination/key-set/write-once tests; route-level 422, hash-rule and
  terminal-status tests. Ruff F401s cleared, OpenAPI regenerated,
  backend/CLAUDE.md updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jcschaff

Copy link
Copy Markdown
Contributor

Findings from using the verify API directly (from compose-api, 2026-10-22), relevant to this GUI and to Novrus's "100% concordance for a run that compared nothing" finding.

1. POST /verify/omex can return COMPLETED with nothing compared

I submitted BioModels' own archive for BIOMD0000000012 with the default five simulators, anonymous, include_outputs=true.

  • Workflow compose-api-verification-3214309a-01f8-4a3d-a0a1-0151276e88c4, run 01a16409-1975-703b-993a-a88da1854808.
  • After about a minute it returned workflow_status: COMPLETED, workflow_error: null, and empty sims_run_info, comparison_statistics and sim_run_data.
  • A client can't tell this apart from a success. That's the state where the concordance figure shows 100%.

2. Why: every simulator run failed, and the failures were dropped silently

I reproduced one child by hand: COPASI 4.45.296 on the same archive, submitted to api.biosimulations.org/runs exactly as run_biosim_sim does.

  • Run 6ada2269041d31202094ca8f ended FAILED, and its run record's error is empty.

  • The reason is only in GET /logs/{run_id}:

    ValueError: /root/archive.omex is not a valid COMBINE/OMEX archive. Content element BIOMD0000000012-biopax2.owl is invalid. Location is not a file. …

    BioModels' archives list files in their manifest that the zip doesn't contain. BioSimulators validates strictly, so all five simulators fail at once. The verify API itself is working.

  • OmexVerifyWorkflow then continues past every child with no run, so the output keeps neither the run ids nor the reasons.

  • Legacy POST /runs isn't auth-gated: an empty body gets 415, not 401. So this isn't a credentials problem.

3. Backend fix: a PR to follow from branch fix/verify-report-failed-simulators

It's on top of #136 (my branch already contains it) and is additive to the contract.

  • VerifyWorkflowOutput.failed_simulators is a new field, a list of {simulator_version, biosimulations_run_id, error_message}, one per requested simulator that produced no results, in request order.
  • The error message is read from the run's logs, from the archive-level exception, else the first SED document's or task's. biosimulations.org leaves the run's error empty.
  • When no requested simulator produced results, the workflow ends FAILED, with workflow_error set to "No requested simulator produced results (copasi:4.45.296: ValueError: …; …)". It still runs the statistics activity, so in-flight workflows replay deterministically. feat(verify): link OMEX hash to verification runs and return run metadata #136's write-once ledger then records FAILED, not a misleading COMPLETED.

Suggestions for this GUI:

  • VerificationStatus.vue already shows workflow_error on FAILED, so the zero-results case surfaces with no extra code.
  • It would help to list failed_simulators even on COMPLETED, since some simulators can fail while others succeed. Show each one's error, with a link to /runs/{biosimulations_run_id} when it's set.
  • The concordance figures should exclude the failed simulators and say how many were excluded, rather than showing 100% over the ones that remain.

4. POST /verify/runs works as documented

BIOMD0000000012's two existing legacy runs, AMICI 0.11.25 (6471071749c0d898ffdd8e26) and COPASI 4.36.260 (6471071649c0d898ffdd8e1e), compared close on all 36 report variables and all 7 plot variables.

5. Context: compose-api will use this as an oracle

The plan is docs/plan-verify-oracle.md in compose-api.

  • 275 of the first 300 curated BioModels entries already have a biosimulations.org project named BIOMD<id>_<simulator>_<title>. compose-api now attaches those runs' results as references (compose-api#342).
  • It will send its disagreeing and failing SED-ML comparisons to /verify/omex as a second opinion.
  • Before submitting, compose-api will strip manifest entries whose files are missing, which sidesteps item 2 for BioModels archives.
  • Once this GUI lands, compose-api's comparison pages can link to it for an archive's verifications, using feat(verify): link OMEX hash to verification runs and return run metadata #136's ?omex_hash= filter.
  • Note for feat(verify): link OMEX hash to verification runs and return run metadata #136's follow-on: the planned size cap on inline include_outputs data affects how compose-api reads BioSimulators' series. A separate way to fetch them would help.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@NovrusShehaj
NovrusShehaj marked this pull request as ready for review October 22, 2026 15:10
@NovrusShehaj

Copy link
Copy Markdown
Collaborator

@jcschaff Thanks, this is a clear diagnosis, and it explains the empty reports. A status update first, then two corrections and four questions for your fix/verify-report-failed-simulators PR.

#126 status: I merged current main into this branch (347cfa9), which brings in #136, #151 and #153, and took the PR out of draft. #136's contract matches this PR's models/verification.ts field for field. The GUI keeps a fallback for backends that list bare IDs, so the order the frontend and backend deploy in doesn't matter.

Confirmed

  • Points 1–2. OmexVerifyWorkflow continues past any child that produced no run (omex_verify_workflow.py#L76-L77). generate_statistics then drops every run without an HDF5 file (runs_verify_workflow.py#L96-L97). The workflow then always ends COMPLETED (#L85).
  • workflow_error on FAILED is shown with no extra code (VerificationStatus.vue#L314-L319). The results dashboard only renders on COMPLETED, and the listing card will read "Failed" once your fix records it. A link to /runs/{biosimulations_run_id} would land on the frontend's existing run page.

Corrections

  1. This exact case didn't show 100%. On ec4be26 it showed 0%.
    • overallConcordance returned 0 for an empty matrix (verify-model.vue#L1114). The false green 100% came from one surviving simulator, or from every pair erroring (#L1129).
    • Both are fixed in this PR: summarizeConcordance returns null when nothing was compared (useVerificationAnalytics.ts#L162).
    • I loaded your exact output (COMPLETED, empty sims_run_info, comparison_statistics and sim_run_data) into the current head in headless Chrome. The summary reads "—" and "Solvers Compared 0", with a "Nothing to compare" notice.
  2. compose-api links need the hash of the archive actually submitted.
    • If compose-api strips missing manifest entries before submitting, the stripped archive has a different MD5 from BioModels' original. A ?omex_hash= link built from the original will find nothing.
    • Use the hash /verify/omex computed from the upload: it's in the listing row, and /compatibility/check returns the same value as omex_id. The link is /utilities/verify-model?omex_hash=<md5>.
    • Anonymous submissions are ownerless, so anyone can see them.

Questions for the failed-simulators PR

  1. Replay safety of the log fetch. Ending FAILED after the statistics activity adds no workflow command, so that part replays fine. If the parent workflow fetches /logs/{run_id} in a new activity, though, workflows in flight at deploy time will hit NonDeterministicWorkflowError on replay. That needs workflow.patched or a worker drain (see backend/CLAUDE.md → Deploy). Alternatively, fetch the logs inside the child's existing activity. Where does the fetch live?
  2. Threshold. FAILED only when no simulator produced results leaves a gap. With one survivor, the run still ends COMPLETED with nothing compared, which is just as indistinguishable from success for compose-api. Should the rule be fewer than two? The GUI already covers that case with the notice above, so this mainly matters for API clients.
  3. Shape of failed_simulators. Is simulator_version the id:version string the request takes, or the BiosimulatorVersion object that sims_run_info uses? Which fields are nullable? Pinning that in the regenerated OpenAPI spec lets the GUI type it directly.
  4. Verifications that already ended this way. Their Temporal output says COMPLETED, so the first GET /verify/{id} after feat(verify): link OMEX hash to verification runs and return run metadata #136 records COMPLETED permanently under the write-once rule, even after your fix ships. Is that acceptable, or is a one-off correction worth it for those rows?

GUI follow-up

I agree with both suggestions and will do them in one follow-up once your PR is up:

  • failed_simulators: show each one's error and a run link, including on COMPLETED. It replaces the sessionStorage workaround (verify-model.vue#L1018), which only works in the tab that submitted. The dead succeeded filter (#L947) goes at the same time.
  • Concordance: failed simulators are already absent from the matrix. I'll count their pairs as uncompared, so the existing "N of M pairs compared" note and the never-green-on-partial-coverage rule (#L1106) apply. For example, 3 of 5 simulators succeeding would read "3 of 10 pairs compared" in amber, not a green 100%.

@NovrusShehaj
NovrusShehaj requested a review from jcschaff October 22, 2026 15:17
@HarrisonCreates
HarrisonCreates merged commit 4edd6b7 into main Oct 22, 2026
5 checks passed
@HarrisonCreates
HarrisonCreates deleted the feature/verification-api-gui branch October 22, 2026 19:48
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.

3 participants