Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the live-simulation equilibrium/Kd plotting and related UI flow, including adding competitive binding (Ki) support and a few UI/parameter tweaks.
Changes:
- Rename the precomputed simulation name→type map for clarity and tweak live binding parameters.
- Update equilibrium plot logic to support competitive binding (Ki) with different fit/annotation behavior.
- Small UI/UX updates (enable navigation button, add quiz hint text, lock page scrolling, reset additional analysis state).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/simulation/PreComputedSimulationData.ts |
Renames static map used to derive agent/product types. |
src/simulation/LiveSimulationData.ts |
Adjusts binding parameter (kOn) and formatting. |
src/simulation/BindingSimulator2D.ts |
Formatting-only changes (trailing commas). |
src/index.css |
Enables overflow: hidden on body. |
src/content/LowAffinity.tsx |
Enables nextButton at end of Low Affinity content. |
src/components/quiz-questions/KdQuestion.tsx |
Adds additional hint paragraph for answering the question. |
src/components/plots/EquilibriumPlot.tsx |
Adds module-dependent regression/annotation logic (Kd vs Ki). |
src/App.tsx |
Expands analysis reset, minor navigation behavior change, and adds a debug log. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <p> | ||
| If you're not sure, look at the where the line crosses the 50% | ||
| mark on the <strong>Equilibrium concentration plot.</strong> | ||
| </p> |
There was a problem hiding this comment.
The new helper text has a grammatical typo: "look at the where" should be "look at where".
| } | ||
| const longestAxis = Math.max(viewportSize.width, viewportSize.height); | ||
| const startMixed = sectionType !== Section.Introduction; | ||
| console.log("NEW BINDING SIMULATOR"); |
There was a problem hiding this comment.
Remove the debug console.log; this will spam the console every time the simulator memo re-runs (module/page/viewport changes) and can impact performance and observability in production.
| if (module === Module.A_B_D_AB) { | ||
| bestFit = regression.exponential(regressionData); | ||
| const max = Math.max(...y); | ||
| const min = Math.min(...y); | ||
| const halfMax = (max - min) / 2 + min; | ||
| // for exponential, the equation is in the form y = a * e^(b*x) | ||
| // bestFit.equation[0] is a and bestFit.equation[1] is b, so to solve for x when y is halfMax: | ||
| // halfMax = a * e^(b*x) | ||
| // halfMax / a = e^(b*x) | ||
| // ln(halfMax / a) = b*x | ||
| // x = ln(halfMax / a) / b | ||
| value = | ||
| Math.log(halfMax / bestFit.equation[0]) / bestFit.equation[1]; | ||
| } else { |
There was a problem hiding this comment.
When module === Module.A_B_D_AB and there are no recorded points yet, y can be empty here. In that case Math.max(...y)/Math.min(...y) evaluate to -Infinity/Infinity, making halfMax and the derived value become NaN. Consider guarding this branch (and skipping regression) until you have enough data points.
| const kdIndicator = { | ||
| x: [bestFit.kd, bestFit.kd], | ||
| x: [bestFit.value, bestFit.value], | ||
| y: [0, fixedAgentStartingConcentration / 2], | ||
| mode: "lines", | ||
| name: "", | ||
| hovertemplate: `Kd: <b>${bestFit.kd.toFixed(2)}</b> ${MICRO}M`, | ||
| hovertemplate: | ||
| module === Module.A_B_D_AB | ||
| ? `Ki: <b>${bestFit.value.toFixed(2)}</b> ${MICRO}M` | ||
| : `Kd: <b>${bestFit.value.toFixed(2)}</b> ${MICRO}M`, |
There was a problem hiding this comment.
For the competitive binding module (Module.A_B_D_AB), the horizontal reference line is based on Math.max(...y) / 2 (50% inhibition), but the vertical indicator line still uses fixedAgentStartingConcentration / 2 for its y-extent. If max(y) differs from the starting [A], the indicator line won’t intersect the selected 50% inhibition line. The indicator’s y-range should be derived from the same target value as the chosen horizontal line in this module.
| let bestFit; | ||
| let value; | ||
| if (module === Module.A_B_D_AB) { | ||
| bestFit = regression.exponential(regressionData); | ||
| const max = Math.max(...y); |
There was a problem hiding this comment.
Inside the useMemo, let bestFit; shadows the outer const bestFit = useMemo(...). This makes the code harder to follow and easy to misuse during refactors. Consider renaming the inner variable (e.g., fitResult) and/or returning a more explicitly typed object.
Problem
Estimated review size: medium
What is the problem this work solves, including
Link to story or ticket
Solution
What I/we did to solve this problem
with @pairperson1
Type of change
Please delete options that are not relevant.
Change summary:
Steps to Verify:
Screenshots (optional):
Show-n-tell images/animations here
Keyfiles (delete if not relevant):
Thanks for contributing!