Repository navigation
Conversation
With the drawing split into components, each one is now built and solved as its own Mechanism: its own mobility, its own input, its own cycle. A half-built chain in the corner no longer takes the finished linkage beside it to 2 degrees of freedom and stops both. The dangerous part was not the split but the pairing that survived it. applyPose walked the whole drawing's joints and read frames.joints[step] at the same array position, which was only ever right because one Mechanism held everything in the same order. Handed a component, that pairing silently places one machine's joint on another's coordinates -- no error, just a linkage that comes apart when you press play. Poses, traced paths and cylinder reach are now matched by id, and each machine is asked for its own frames. Time is shared but cycles are not, so the clock follows the longest running cycle and a shorter machine wraps inside it. Where "is this simulatable" used to be one question about one mechanism, it is now two: any machine ready (the gate for analysis) or the particular machine a part belongs to. A chain that never reaches ground stops being built as an invalid mechanism and becomes unassigned geometry instead, which is what changed under three fixtures that had quietly relied on a Mechanism always existing.
isMechanismValid() answered one boolean for seven quite different situations, and the panel could only pass that vagueness on. A linkage whose mobility is 2, one locked at a dead position, one whose driven joint cannot be driven and one whose barrel is too short to slide in all arrived as the same nothing-happens. The solver already distinguished them -- it just threw the distinction away at four separate call sites. Each now records why, and the reasons are ordered the way the fixes depend on one another rather than by severity: a slider with nothing to slide along has no mobility worth counting, and giving an input to a linkage whose mobility is wrong will not make it run. A list a student works down should not send them to do something that cannot help yet. Each check names the way out, and where there is a part at fault it carries that part so the panel can offer to go to it -- pointing, for a missing input, at a joint that could actually take the job rather than at the mechanism in general. Grashof is deliberately absent. It classifies four-bars only, and the app can already say from the solve itself whether an input completes a revolution or reverses at a toggle, which is both more specific and true of any linkage. Splitting the drawing also made one of these rarer: a detached slider floating beside a good four-bar used to drag it down, because both were one mechanism. Now the block simply never reaches ground, and the four-bar is left alone. The cylinder-with-no-travel case had been reading a solver static after the fact, which with several mechanisms would have handed one machine's complaint to another; each now captures its own at build time.
Speed was one number for the drawing, which stops being a speed at all once the drawing holds several machines: a cylinder beside a crank would be handed the crank's rpm. It now rides on the driven joint, because that is the only handle on a mechanism the URL carries -- names are derived from the geometry rather than saved, so there is nothing else stable to key on. It is also where the panel has always shown the setting, so nothing moves for the reader. Zero means "follow the document-wide default", and a joint running at that default writes no token at all. Every template and every previously shared URL therefore still re-encodes byte-identically, and only a drawing that actually uses the feature pays for it. A speed with no slot writes the slot triple empty, without which the decoder would read the speed as a carrier id. Selection leaves the URL entirely. It was two things it should never have been: part of what a shared link says, so opening someone else's mechanism selected whatever they had last clicked; and part of the undo history, since undo is URL replay -- pressing it after clicking a joint moved the highlight instead of the mechanism. The field stays, always empty, because its position in the format is load-bearing. What was selected is now simply re-found among the rebuilt objects, so a selection survives an undo without ever having been stored.
The gripper stopped animating. Its mobility was still one and its mechanism still built, so nothing complained -- it simply solved something else, and the arms wandered off and never closed their cycle. Its sliders run in slots cut into anchored bars: links whose joints are all grounded, so they *are* the world and have no body of their own for a joint to connect. A slot's carrier and the two joints drawing its line live outside `links` and `connectedJoints`, so the component walk never reached them and each mechanism was handed its sliders without the rails they run on. Four grounded frame joints were also being reported as having no link at all, which they plainly do. Following those references closes it, to a fixed point because a carrier can itself carry a slider. Caught by the gallery sweep rather than by the unit suite, which is the argument for running it: 1890 graph checks and every timing check passed while this was broken.
A horizontal file toolbar and a vertical mode rail took a whole edge and a corner of the window between them, to show five buttons nobody presses more than once a session and three modes that are the whole app. The modes take that space now, in a strip of floating cards, and New, Open, Templates, Save, Share, Settings and Help fold into one menu behind the logo. Analyze splits into Kinematic and Force, because they are different questions with different requirements and one tab could only ever answer for both by being vague. Each analysis mode carries a chip saying what stands between the drawing and that answer -- and stays pressable when it cannot be entered. Greying it out provokes exactly the question "why not?" while being the one control unable to answer; pressing these opens the list instead. Force analysis reports what the force solver itself refuses, rather than a second opinion free to drift from it. The mock this follows claimed cylinders were the obstacle; they are not. A welded slide is, because a prismatic pair carrying a moment has no column in the equilibrium model. The rail's sliding highlight came along, same 200ms and easing, now sliding sideways. It has to be measured off the button after the pass that marks it active, not during: measured inside that pass it reports where the highlight used to be, and trails a tab behind.
Pressing an analysis mode that cannot be entered now opens the drawer that says why, one section per mechanism, each blocker naming the way out. Before this the answer was a single sentence about the whole document with first-blocker-wins -- which in a drawing holding several machines is a sentence about whichever the loop reached first -- and for a mode that simply did nothing, often no answer at all. The drawer answers the question that was asked. Pressing Force when force analysis is short of something shows what *force analysis* wants, not the kinematic blockers, which are a different list with different fixes. Both are shown, so a reader refused by one mode can see the shape of the other, but only while something is outstanding: a met requirement is a tick nobody needs to read twice, and a column of green buries the one line that matters. Where a check is about a particular part it offers to go there, and going there is deliberately not an edit. Being shown where a problem is has not changed the mechanism, so Undo afterwards still takes back the last real change rather than the last time the reader looked at something. The drawer floats clear of the strip above it. It used to start under a toolbar that ran the width of the window and could sit flush to the edge; nothing is flush any more.
Deleting the mode rail took the animation bar with it, so until this there was no way to press play at all. It comes back as cards over the grid: speed and play on one, the scrubber and a row per mechanism on another, the view controls beside them. A row exists only for a machine that can actually run -- a disabled scrubber for a linkage with no cycle is a control that can only disappoint, and what to do about it belongs in the setup drawer. Neither mobility nor readiness appears beside a row, because being listed is what ready means and a word that can never read anything but yes is not worth the space. The bottom bar becomes a status strip: the mode, and what the mode means for the mechanism right now. It reports and nothing more -- pointer events off, so a line that looks like text cannot turn out to be a button. The mode highlight took three attempts because two plausible diagnoses were both wrong. It was not the measurement being cancelled by a busy change-detection loop, and it was not the animation frame running outside the zone. The field was correct the whole time; the view simply never re-rendered after the frame assigned it, because that assignment lands after the pass that would have shown it. Reading the component's own state rather than guessing from the screen is what found it.
Kinematic and force analysis sat one above the other in a single panel, so every reader scrolled past the answer they had not asked for, and neither question could state what *it* needed -- the panel could only be vague enough to cover both. They are separate modes now, and the panel shows the half the mode is asking about. Every graph is where it was; only the question changed. Four panel specs were asserting both halves at once, which is exactly the thing that stopped being true. Each now says which question it is asking, and the one that counted "force rows plus two kinematic graphs" counts the force rows and the input effort -- the kinematic graphs belong to the other mode, and to the test below it.
The read-only guard named the kinematic tab specifically, which was right when that was the only way to analyse anything. Force analysis reads the same solved cycle and is no more able to survive the geometry moving under it, so both modes are covered now. The status strip says so, because a rule that silently refuses is worse than one written down.
The per-mechanism speed had a place to live and no way to reach it: the Input Settings field still wrote the document-wide number, so a drawing with two machines could only ever drive both at once. The field now writes the speed of the joint it is showing, and the document default follows the last speed set -- so a newly driven joint starts where the reader last was, and a drawing holding one mechanism behaves exactly as it did when the speed really was one number. The direction button flips that joint's drive rather than the document's. One definition of "how fast does this joint drive", read by the solver and shown by the panel, because two would eventually disagree and the disagreement would show as a panel reporting a speed the mechanism is not running at.
A reader told their linkage is ready still has questions the app answered nowhere: which joint drives it, how fast, how long a cycle takes, whether it goes round or backs up. Those facts now sit under each mechanism in the setup drawer -- which is where someone goes to ask about a mechanism, so they did not need a new place to be selected from to have somewhere to live. A healthy mechanism stays collapsed to its name and its Ready chip; the facts are one press away rather than in the way. "Motion" was reciprocating for everything at first. It compared a joint's position at the start of the cycle against the midpoint, and the joint it sampled was the first in the frame -- which is usually ground, and ground never moves. The solver already flips the sign of the recorded input velocity at each reversal, so a cycle holding both signs is exactly one that turned around, and that is what both this and the transport now read.
Every mechanism ran on one wall clock, which is the right default -- two machines on the same clock can be compared, and comparing them is most of the reason for drawing them together -- but it was also the only option, so reading one machine's cycle meant watching the others move. Unsync hands each its own time and its own play button. Leaving sync gives every machine the time it is already showing, so nothing jumps at the moment the toggle is pressed. The toggle only appears when there is more than one machine to get out of step, which is the same rule the rows follow: the transport shows what there is to control and nothing else.
A review of the branch found these; two are mine from this week's work and the rest were uncovered by the split. The dangerous one: a rebuild deep-copies the editable joints as t = 0, and `restoreStartPose` put them back there by asking `applyPose` for time zero. Once each machine had a clock of its own, applyPose honoured *those* instead and left every mechanism wherever its own scrubber was -- so an edit made while a row was scrubbed forward silently redefined the start pose, and the geometry ratcheted on every subsequent edit. It now places each machine at its own zero directly, and "are we at the start" asks every clock rather than only the shared one. One input per *mechanism*, not one per drawing. Driving a second linkage still cleared the first one's input, so two machines could never run at once however well the solver handled them. A fixed bar joining two frames handed its far end to both partitions, and that end could be another machine's driven joint -- so a mechanism picked up an input that drives something else. A partition now distinguishes what it is made of from what it merely has to be solved against, and every question of the form "which of these is mine" asks the former. A bar grounded at both ends belongs to no machine, which is true, but it was reported as joints with no link and the reader was told to attach one to something that already had it. It is now named for what it is. The transport's direction button reached for the rotational speed to supply a prismatic joint's fallback, storing 20 cm/s where 5 was meant. Force analysis asked whether *every* link in the drawing had mass and whether a force existed *anywhere*, so an unrelated massless bar blocked a good mechanism and a load on a broken one satisfied the requirement.
Analyze split into Kinematic and Force, and a drawing can hold several mechanisms. The vocabulary guide governs what the app says, so it has to say the same thing.
Both were reproduced in a browser by the reviewer and fixed without a test, which is the wrong way round for a rule this load-bearing: a shared frame bar handing one machine another machine's driven joint, and a bar fixed at both ends reported as joints with no link. Each fails on the pre-fix code, and on the right assertion.
Two defects an e2e sweep found. animate() applied the running flag twenty lines after it announced the new step, so a caller *stopping* playback -- which is what leaving an analysis mode does -- notified its subscribers while the service still believed it was playing. currentTimeSeconds() then answered with the playback clock rather than the time of sample zero, and the readout kept showing the time the mechanism had been left at while the mechanism itself had rewound. The comment at that line already said subscribers read the drawn time back off the service; the flag is part of that. At 390px the top strip ran off the edge and took the page with it, which put two of the four modes somewhere unreachable. The tab card scrolls on its own now, the chips go -- their status is on the status strip anyway -- and so do the units and version at the other end.
✅ Deploy Preview for pmksprod ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
`AnimationBarComponent.animate` outlived the component's own rendering by a whole redesign: the bar stopped being drawn when the mode rail went, but its static remained the app's answer to "is the mechanism moving", so anything needing to know had to import a piece of chrome to ask, and app.module still declared a component that appeared nowhere. The flag is now `MechanismService.isPlaying`, and the bar, its template, its stylesheet and its spec are gone. Two specs had used the static precisely because it needed no instance -- one read it before its service existed, the other reset it from a scope that had none. Each test builds its own service now, so there is nothing global left to restore.
One scrubber per line. The transport card carried a master track *and* every row carried its own, which left the reader to work out which of the two they were being asked to believe. Synced, the machines move together and there is nothing to say about them separately, so the rows collapse to a single All line -- and a row shows a play button only when it is one of several being controlled apart, rather than sitting beside the transport's own button doing the same job. Row play is now the same filled indigo circle as the master, because it is the same control applied to one machine, not a different kind of thing. A reversing machine now runs its handle up the track and back down, which is what its drive does. Reading the handle off position in the sample array sent it round and round -- the samples go out and back as one cycle -- which is precisely what showing the reversal was meant to replace. The row says Reciprocating, and Reversing while it is actually going backwards. The view controls stop being pinned to a corner and join the bottom cluster in analysis, on the mock's animated flex spacer, so they glide across instead of jumping. They are icon-only squares now, which is the button vocabulary the rest of the floating UI uses. Undo and Redo are hidden in analysis rather than disabled: the geometry is locked, so there is nothing to undo. The history card sits beside the modes instead of at the far edge, so the three cards read as one group. The status strip gains the cursor and spells out the whole unit set rather than the length alone. The version leaves it for a footer row in Settings -- permanent furniture answering a question asked twice a year. Three e2e checks asserted the old contract and now assert the new one.
Both were a collapsible section wrapped round an animated GIF of the gesture: something that had to be watched, could not be scanned, and took three variants in Edit to say what one panel says here. They are the mock's panels now, word for word -- a title, a lead, a rule, and a hint or two. Analysis Help says a different thing when something is selected whose mechanism cannot run, because telling that reader to select something would send them round a loop they are already standing in. Defined once in the drawer that hosts both panels rather than twice in the two components that draw it. My first attempt appended the rules to the end of each component's stylesheet, where the closing brace belongs to whatever rule happens to be last -- so they shipped as `.cylinder-clamped .helpTitle` and matched nothing. Reading the served CSS is what found it; the source looked right.
One drawer carried both modes' lists, so a reader refused by Force read past the kinematic blockers to reach the thing that had actually stopped them. They are two drawers now, each answering only its own question, and each says which question that is. The facts about a mechanism leave the drawer entirely -- they belong in the mechanism's own panel, and appearing in both places meant the same six numbers twice. Every drawer closes from its own corner. Its slide-out was also two pixels short of gone: the offset was measured against a panel flush to the window edge, and the drawer floats at a twelve-pixel inset now. The mode chip opens that mode's setup whether or not the mode can be entered. Without it the list was reachable only by being refused -- and a drawing where one machine is ready and another is not is *enterable*, which left the reader with a chip counting problems and no way to read them. Found by a test that could no longer get to the drawer.
A mechanism was the one thing in the drawing that could not be selected, so the facts about it had nowhere to live and ended up in the setup drawer beside the blockers -- the same six numbers in two places, and no way to act on the machine as a thing. Selecting one selects all of it: every joint and link reads as selected, rather than leaving the reader to infer the extent of what they just picked. The panel is the same in both modes because the facts do not change with the question; Edit adds Delete, which takes the machine's joints and everything that cannot stand without them. Two routes in, because the obvious one only exists half the time. The transport chip names a machine, so pressing it selects that machine -- but the transport belongs to the analysis modes, which is why Edit could not be reached at all. The setup drawer's own mechanism name selects it too, and the drawer opens from the mode chip in either mode. Rename is deliberately absent: names are derived from the geometry rather than stored, so there is nothing to rename yet.
The animation bar had drifted a long way. It is rebuilt from the mock's inline styles rather than from an eye on a screenshot, which is how it drifted in the first place: card 0 1 470px / min 250 / padding 9 12 / gap 7, chip 40x30, row play and transport play both filled indigo, the track in a 22px well pinned 9px down so the handle centres on the line, and the time under the track at 11px rather than inline beside it. The sync control is the mock's tab off the top edge of the card, not a footer inside it. The written feedback described a footer; the mock's own syncBtn is position:absolute, top:-26px, radius 10px 10px 0 0 with the shadow above it. I followed the mock, since that is what "match it exactly" points at, and flagged the difference. The mock has no time field at all, only text. Keeping a typed time mattered more than losing it, so it is an input styled to be indistinguishable from that text -- and it now seeds itself, because the position subject only emits when the pose moves and a mechanism sitting at zero showed an empty time. Panels were double-boxed: the drawers drew a card and the panel-sections inside them drew another, so every panel had two blue top borders and two rounded edges. The drawers draw nothing now; the three panels that are not built from panel-sections carry their own card instead. Accordion headers all take the Analysis-setup treatment -- transparent, a hairline rule above, indigo label -- instead of a tinted bar per section striping the panel and competing with the values beneath.
A review pass over the whole app at six widths. It confirmed the bottom cluster now matches -- same baseline across the cards, sync tab and handles aligned, no double borders -- and found four things wrong. A closed drawer still occupied the page. Parked past the right edge it kept its width, so the document was 1592 wide in a 1280 window and the "closed" drawer could be scrolled back into view. It is hidden now, and the document itself is clipped: everything in this app scrolls inside its own panel, so the page never should. A setup drawer outlived its mode. Opening the Force list and switching to Synthesis left it sitting over the synthesis canvas, which reads as an app stuck between two places. Each drawer belongs to the mode that opened it and leaves with it; Settings and Help are about the app rather than a mode, so they stay. The bottom cluster did not fit a phone: the transport ran off one edge and the view controls off the other. It wraps now, with the scrub card taking the width it can get. Left and right panels overlapped below about 900px, interleaving their borders. One panel at a time down there: the drawer wins and the left panel stands down. Two smaller things: the mode labels vanished in two stages, so Synthesis and Edit went icon-only while Kinematic and Force kept their words -- a duplicated rule nested inside the phone block was switching them back on. And the version row sat outside the Settings card rather than in it.
The redesign renamed or removed every element three of the tour steps aimed at, so those steps dimmed the screen and highlighted nothing. Aim them at the new chrome, and drop any step whose target is missing rather than showing an empty spotlight. Alongside: hide the disclosure arrow on a mechanism with no checks behind it, name the icon-only tab and history buttons for screen readers, stop the edit help from offering actions the menu does not have, and settle every panel at one elevation so a nested section no longer reads as a second, higher card.
The heading already knew the selected body was a cylinder, but every row under it went on quoting the barrel link -- "Analysis for Cylinder GC" above "Angle of Link GN", the panel disagreeing with itself about what the reader had selected. The speed note had the same problem from the other end: it sent the reader to "the input joint's Edit panel" for a joint sealed inside the ram, with no hitbox on the canvas and no row in that panel. Name the field and the part that carries it. And read that speed from the mechanism the selection belongs to. The note quoted the first driven joint in the document, so in a drawing with three machines it reported one of them three times.
Leaving an analysis mode rewinds the mechanism, and doing that in one frame teleports a linkage the user was just watching move: the pose they paused on is replaced by a different one with nothing in between, which reads as the drawing breaking rather than as playback ending. Ease over it in about a fifth of a second. This is a seek per frame, not playback -- the solver never runs and it lands on exactly the sample the cut landed on -- and if anything else moves the mechanism part way through, the rewind stops where it is and leaves that caller alone.
The series controls are labelled "X" and "Y" -- nothing on their own, and a panel showing six graphs offers a dozen checkboxes all called the same two things. The plot itself is a canvas with no text in it at all, and the CSV button is one of six identical "Download Data" buttons. Name all three from what the graph is already told it is showing, and name the status chips in the top strip alongside, so the count they carry survives the button having an explicit label. The two overlays that cover the plot -- "select at least one series" and "unavailable while dragging" -- also shared one id between them, so nothing could refer to either.
Below 1080px the mode tabs lose their labels and become four unnamed glyphs. Name them on hover. On the icon rather than the button: the status chip sits inside the button, so a tooltip there fired over the chip as well and two of them came up stacked.
Three things the transport got wrong about direction. The note beside each row said "Reciprocating" -- a fact about what kind of machine it is, repeated twice a cycle. It now says which way the input is actually going: Clockwise or CCW for a rotary drive, Extending or Retracting for a linear one. The handle carries the same fact, running left to right for a clockwise input and right to left for a counter-clockwise one. Reversing a machine stopped it and, on a machine whose input already turns around on its own, mirrored a drive that has no other direction to be driven in. A continuous drive is still reversed by re-solving the cycle; a reversing one now plays backwards instead, which is a view of the same motion rather than a different drawing. Either way it holds its place and keeps running, and the pose it starts from is untouched. The stop was not the transport's doing: encoding the URL round-trips through the start pose and handed back the frame but not the fact that playback had been running. Any edit made while the mechanism moved stopped it. And the master play button no longer disagrees with the rows. It starts and stops all of them, and goes quiet by itself when the last one does.
The menu was one flat list of up to eight equally weighted rows, with Delete at the top next to Attach Link, labels that rewrote themselves as the object changed, and three different ways of saying no: Make Circular was hidden where it did not apply, Attach Link was greyed with no explanation, and Weld was offered and then refused by a snackbar after the click. A prismatic joint got no menu at all, and three of the four modes opened either a two-item menu or nothing. Now: a fixed ladder — Attach, State, Machine, and a destructive footer alone under a rule — so a two-row menu and a twelve-row menu read the same way and the flick to Delete always lands on the last row. A header names the target and carries the way into the other mode as one icon. States are written as ticked switches rather than verbs that flip. One availability rule replaces the three: greyed with the reason beside it where a reader could expect the action, absent where it is structurally impossible for that kind of part. Every reason is fetched rather than restated — describeActuatorRefusal reads the branches of describeActuator back out, weldRefusal does the same for canToggleWeld, and the deletion rows ask deleteLink's and deleteJoint's own cascade rules in advance. Three surfaces quoting one model cannot disagree about what is possible. The decision moved out of the 4,000-line canvas into ContextMenuBuilderService; the canvas supplies only the gestures. New: Go to Edit from either analysis mode and Go to Analysis from Edit (gated per part, not per drawing — a grid can hold a machine that runs beside one that does not), per-joint Trace Path, Duplicate Link, Attach Force at a joint where one link meets, Graph Joint Force, and counts beside Lock All / Unlock All. Prismatic joints get a menu. Wording follows docs/ui-vocabulary.md rather than the mock, and the guide is updated where this settled a question it had left open. "Make Force Global" was its one blessed use of "Make"; it is the Global Frame switch now. Three refusals and all five tutorial steps named rows that no longer exist and were chased down. Bugs found on the way, several by the reviews and several by the tests once they could run again: a floating slider falsely told it had no force to graph; two deletion rows promising less than the deletion does; "locked means undeletable" enforced only by the menu that showed it, on none of the four delete routes; a context card left standing as a snapshot with live keys behind it; Duplicate Link copying values without the flags that make them custom, and holding the copy's centre of mass against the original's joints; the builder throwing outright on a SliderBlock; toggleCurve never handling a prismatic joint; and three icons with no viewBox that cropped below 24px. Reviewed twice by GPT-5.6 sol. A third pass could not run, and a further review after merge is expected. One finding is deliberately left: a driven joint on an unsolvable mechanism is still offered its input graph, because the analysis panel makes the same exception and this row's job is to agree with the panel rather than to be stricter than it. 1,252 unit tests · 41/41 context-menu e2e · 337-action interaction sweep with no findings · 59/59 tutorial e2e · production build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…and stop the transport taking your selection (#246) A collection of small fixes, each found by using the app rather than reading it. **Framing.** The canvas is full bleed and every panel floats over it, so Reset View framed to the window and centred the drawing partly behind the Edit panel with a quarter of the canvas empty beside it. Chrome now declares which edge it takes and the fit measures what is left. Mode changes carry the drawing with the free canvas; the right drawer pushes it left rather than covering it, and gives the view back exactly on closing; a window resize scales the view with it, to the pixel, at any cadence; the zoom buttons zoom about the free canvas rather than a point behind the panel; Reset View on an empty grid returns to the origin; a narrow window goes under the panel and comes out below it rather than being framed behind it. The rule underneath: a view is either where a fit put it or where somebody drove it, and a driven view is remembered and given back rather than taken away. Drawn marks get a size to suit the mechanism when nobody has chosen one -- the wiper lands on the 0.7 it already had, the Jansen leg on 6.79 against the 7 its author chose. **Colour.** A joint can be given one of four families (amber unchanged as the default, orange, deep orange, brown), each a set of three so it reads as one object resting, hovered and picked; a selected joint keeps its own fill and wears amber inside its edge, so the fill says which joint and the ring says it is the selected one. Forces get the six colours the links are drawn in. Both travel in the URL, because undo replays those strings. **Transport and synthesis.** Play, direction and the scrub handle no longer take your selection -- the row's whole surface selects the machine, and they were inside it. A synthesis design can be typed: all nine boxes are live from the start, a half-typed row is not thrown away, and the canvas follows a position typed outside the view. The length grip sits on the end away from the fixed reference. Two bugs found on the way: the reframe was permanently stripping `svg-pan-zoom_viewport` off its own viewport element, and the library applies a new matrix a frame later than the call, which made measure-move-measure lie. 1277 unit tests, 113 synthesis checks, and the framing behaviours measured in a browser rather than assumed. Reviewed six times by GPT-5.6 sol across the branch's life, which found fourteen real problems, all fixed and re-verified.
|
[P1] Derive force-preview units from the selected system and effort kind
|
|
[P2] Label the third reaction series as magnitude, not Z
|
|
[P2] Make the analysis empty-state hint mode-specific
|
|
[P2] Make the setup chip a separate keyboard-operable control
|
KohmeiK
left a comment
KohmeiK
left a comment
There was a problem hiding this comment.
Deep review — 14 finder angles, adversarially verified
Automated review (Claude Code, extra-high recall) of the full staging→branch diff (409 files, +53k/−10k), run as 14 independent finder passes (line-by-line per area, removed-behavior audit, cross-file tracing, language pitfalls, cache/copy correctness, reuse/simplification/efficiency/altitude lenses, conventions, UX copy, and a live Playwright sweep of the deploy preview), then every candidate re-verified by an adversarial pass that tried to refute it against the code at HEAD. 86 candidates went in; the 66 inline comments below are what survived, each tagged [category · confirmed/plausible] — confirmed means the trigger and wrong output were traced end-to-end with quoted lines; plausible means the mechanism is real but the trigger runs through a race or rare state. Refuted candidates were dropped (e.g. an NG0100 claim in the mass table, a paint-delay claim — the export pipeline is properly chunked, and the CoM-anchor drag path is actually correct — though tracing it surfaced a real decode-time bug, commented on mechanism-builder.ts).
Recurring themes, if you want to fix families rather than instances:
- Per-machine vs document-global drive state — the biggest cluster:
setDriveSpeedmoving the default without pinning, thedrivenJointgetter missing slider/cylinder resolution, the direction label/canvas arrows reading globalisInputCW, and units never rescaling linear speeds. partition.jointsvsownJointsfor "which input drives me" (mechanism.ts:142,inputVelocityFor, readinessfactsOf), plus no input reconciliation when machines merge.- Per-machine clock state across rebuilds — arrays never truncated, partition id = index, the zero-skip in
restoreOwnTimes, and thesettings.animatingmirror; one truncate/realign inupdateMechanismplus a model-owned at-rest predicate closes most of it. - The ~220 ms ease window — several gates (
massEditable,unitsEditable, Ctrl+Z) disagree with the canvas/buttons exactly there.
Checked and clean, so you don't re-litigate: old-URL byte-compatibility of the transcoder (new sections are optional, fail-closed, checksum-covered; empty-write rule holds), weld/unweld graph restoration, the Mechanism deep-copy identity wiring, the export writers' xml/CRC plumbing, loop-solver backtracking, drive-profile sign algebra, tutorial step derivation, and the context-menu refusal quoting. The live sweep also verified transport rows, stop-to-start exactness, drawer shift/restore, reset-view framing and zero console errors on the deploy preview.
One note that couldn't anchor to the diff: src/app/model/drop-target.ts's merge refusals say "attach at its mounts" — docs/ui-vocabulary.md calls mount a code word, and nothing on the drawing is labelled one — but that file is untouched legacy, so it's a pre-existing nit for the same copy sweep as the vocabulary comments.
🤖 Generated with Claude Code
| if (joint) { | ||
| // This mechanism's drive, not the document's. A drawing can hold several | ||
| // and turning one round must leave the others turning as they were. | ||
| this.mechanismService.setDriveSpeed(joint, -this.mechanismService.driveSpeedOf(joint)); |
There was a problem hiding this comment.
[correctness · confirmed] Flipping or typing one machine's speed silently changes every machine still on the document default.
setDriveSpeed (mechanism.service.ts:389–401) writes the joint and the document-wide defaults (inputSpeed/linearInputSpeed/isInputCW.next(...)), and driveSpeedOf falls back to those defaults for any driven joint with driveSpeed === 0 — the state every pre-existing URL and every freshly-driven joint is in. reverseDrive (:3970) calls pinDriveSpeeds() first and its comment names this exact failure, but both Edit-panel callers (flipInputDirection here, and the speed commit at :1114) skip the pin. Repro: two four-bars on the default, flip M1's direction → M2 reverses on the same rebuild; type a speed for M1 → M2's rpm changes too.
Fix: pin other machines' speeds before moving the default, as reverseDrive already does.
| @@ -81,9 +173,22 @@ export class EditPanelComponent implements OnInit, AfterContentInit, OnDestroy { | |||
| label: option.label, | |||
| })); | |||
|
|
|||
| /** The joint whose drive this panel is editing, if it is editing one. */ | |||
| private get drivenJoint(): RealJoint | undefined { | |||
There was a problem hiding this comment.
[correctness · confirmed] The drivenJoint getter never resolves a driven slider's pin → PrisJoint (nor a cylinder's slider), while the template's own guard (isVisuallyInput, which does hop via getSliderJoint) shows the Input Settings section anyway. So for prismatic drives the section renders but every handler misses the machine:
- Speed commit falls to the
isSliderInputbranch and writes onlysettings.linearInputSpeed— a pinnedPrisJoint.driveSpeedis never written, so the machine keeps its old speed while the box shows a new number… briefly:patchInputSpeedFieldthen readsdriveSpeedOf(undefined)= the rotational default and displays it beside thecm/sunit label. - Direction flip falls to the global
isInputCW.next(!value)— reversing other machines (see comment at line 188) and not this one. - Cylinder case: the cylinder panel wires these same handlers, but
selectedJointstill holds whatever joint was clicked before — typing an Expansion Speed can callsetDriveSpeedon a stale crank in another machine, with a cm/s magnitude fed in as RPM.
Fix: make drivenJoint resolve the pair the way adjustInput does (via getSliderJoint), and resolve a selected cylinder to its sealed.slider.
| @@ -347,6 +712,10 @@ export class MechanismService { | |||
| link.massMoI *= inertiaScale; | |||
There was a problem hiding this comment.
[correctness · confirmed] updateLinkageUnits converts coordinates, mass, MoI, CoM and force magnitudes — but not linear drive speeds: neither a prismatic input's Joint.driveSpeed nor SettingsService.linearInputSpeed, both stored in user length units per second and consumed unconverted by inputVelocityFor (signed * MODEL_SCALE). The settings doc even says the unit "follows lengthUnit" — the label re-spells, the value never rescales.
Repro: slider driven at 2 cm/s, switch length unit cm → m. Geometry divides by 100, speed stays 2 → effectively 2 m/s: the cycle period collapses ~100×, every velocity/acceleration graph is off by 100×, and the panel still reads "2". RPM drives are unaffected (angular, scale-invariant), which hides the bug from the common case.
Fix: scale linearInputSpeed and every pinned prismatic driveSpeed by lengthScale here, exactly as forces go through forceScale.
| return j.input; | ||
| }) !== -1 | ||
| ) { | ||
| const driven = this._joints[0].some((j) => j instanceof RealJoint && j.input); |
There was a problem hiding this comment.
[correctness · confirmed] Driven-joint discovery runs over partition.joints — which the world-bar pass deliberately pollutes with joints borrowed from other machines — instead of partition.ownJoints. Same slip at inputVelocityFor (mechanism.service.ts:353) and factsOf (readiness.ts:254); drivenRefusal, inputAngleDegrees and readiness.ts:219 already use ownJoints correctly. mechanism-partition.ts:109–114 documents this exact trap, and the fixture at mechanism-partition.spec.ts:153 proves a driven pivot on a shared world bar lands in the neighbour's partition.joints.
Constructible in one UI route: attach a bar from a four-bar's ground pivot to a new joint, ground it, right-click → Driven Input (canDrive accepts). The neighbour machine then skips the not-driven blocker, is handed the foreign speed, and reports "Nothing moves when the input turns" + names the foreign joint as its "Driven joint" — instead of "Nothing drives this mechanism". Worse: because the toggled joint is unowned (index −1), the clear-inputs fallback scopes to all joints and wipes the four-bar's own input in the same click.
Fix: hand ownJoints to the Mechanism constructor (or filter _joints[0] by ownership) and use ownJoints at the other cited sites.
| @@ -869,7 +1663,9 @@ export class MechanismService { | |||
| } | |||
There was a problem hiding this comment.
[correctness · confirmed] One-input-per-machine is enforced only at toggle time (clearInputsSharingMechanismWith); no merge path reconciles inputs when a structural edit joins two driven machines. Attach Link cross-machine passes the only check (commonLinkCheck), and mergeJoints propagates target.input ||= source.input (:1588). Un-grounding a shared pivot has the same effect.
Repro to a legal DOF-1 state: four-bar driven at A + separate driven dyad E–F–G; drag G onto the four-bar's coupler tracer. Result: a Watt six-bar with two input=true joints. The solver drives whichever findIndex returns first; the other joint keeps its badge and its Edit-panel speed controls silently do nothing; and since isInput is encoded per joint, the contradictory state round-trips URLs and undo forever.
Fix: after any merge/structural edit that fuses partitions, keep the first input per machine and clear the rest (or surface a blocker naming both).
| } | ||
| const reader = new FileReader(); | ||
| reader.onload = () => { | ||
| this.notify.success('file.loaded', 'Mechanism loaded.'); |
There was a problem hiding this comment.
[correctness · confirmed] The Open flow toasts success before decoding: reader.onload runs notify.success('file.loaded', 'Mechanism loaded.') (here) and only then urlProcessor.updateFromURL(...) (:478). updateFromURL catches decode errors internally and raises its own failure — so opening a corrupt or old-format .pmks file shows a green "Mechanism loaded." immediately followed by a sticky "could not be opened" over an unchanged drawing. Move the success after the decode (or let the decode own it).
| await page.locator('.stepBar').first().click(); | ||
| await page.waitForTimeout(400); | ||
| check('the progress bar jumps to a step', (await stepNow())?.step === 1); | ||
| while (!(await page.locator('.stepArrow').last().isDisabled())) { |
There was a problem hiding this comment.
[test-coverage · confirmed] Unbounded wait loop: while (!(await …isDisabled())) { click; waitForTimeout(260) } has no iteration cap or deadline. A regression in the arrow-disabling logic (or the locator matching a different arrow) makes the script click-and-wait forever with no diagnostic — the exact unbounded-poll pattern the rest of these scripts avoid (fixed waits or page.waitForFunction, which carries Playwright's default timeout). Cap it (e.g. for (let i = 0; i < 30; i++) + a loud throw on exhaustion).
|
|
||
| describe('a link drawn as a circle', () => { | ||
| beforeEach(() => { | ||
| SettingsService._objectScale.next(1); |
There was a problem hiding this comment.
[test-coverage · plausible] beforeEach sets the process-wide SettingsService._objectScale subject to 1 with no afterEach restore (same in link.com-anchor.spec.ts:62). That subject is module-level state shared by every spec file the worker runs, default 140 — so any later file rendering geometry at the default scale sees 1 instead: an order-dependent failure. The repo already documents this exact hazard and restores it properly in cylinder-lifecycle.spec.ts:11–21 and fixture.ts's try/finally — these two specs just missed the pattern.
| @if (hasStatus()) { | ||
| <span | ||
| class="chip" | ||
| role="button" |
There was a problem hiding this comment.
[ux-a11y · plausible] The readiness chip is a <span role="button" (click)=…> nested inside the mode's <button class="tabButton"> (Kinematic :73–82, Force :101–111) with no tabindex or key handler. Interactive descendants of a button are invalid HTML/ARIA — screen readers flatten them into the parent's name — and the chip only answers the mouse, so once a mode is ready and entered, a keyboard user has no way to open its requirements list from the chip (the design comment says the chip exists precisely because the list is otherwise "reachable only by being refused"). Consider making the chip a sibling control, or giving the tab button a secondary keyboard affordance.
| import { waitForReady } from './app-ready.mjs'; | ||
| import { readFileSync, mkdirSync } from 'node:fs'; | ||
|
|
||
| const BASE = process.env.BASE ?? 'http://localhost:4310'; |
There was a problem hiding this comment.
[conventions · confirmed] New e2e scripts drift from e2e/README.md's contract: this script and pointer-pairing.mjs:19 read process.env.BASE, so the documented PMKS_BASE_URL=… is silently ignored (the README documents PMKS_BASE_URL — app URL (default http://127.0.0.1:4200)); and locking.mjs:15 (4700), notifications.mjs:20 (4600), background-image.mjs:16 (4710) default to ports no documented server runs on — following the README verbatim gets ECONNREFUSED instead of a test run. Standardize on PMKS_BASE_URL with the documented default.
|
[P1] Convert linear drive speeds with the length unit
|
|
[P1] Initialize decoded CoM anchor offsets before the first rebuild
|
|
[P1] Convert the persisted synthesis design when units change
|
|
[P2] Check the selection type before asking for the selected object
|
|
[P2] Honor Delete for every selectable object that advertises it
|
|
[P2] Give the Project menu real keyboard/popover behavior
|
|
[P2] Keep background-image edit handles in Edit mode
|
|
[P2] Include a tutorial-only drawer in the derived drawer state
|
|
[P2] Give each playback scrubber an accessible name
|
|
[P2] Start background-image gestures only with the primary button
|
|
[P2] Rebuild the context-menu target on the background edit surface
|
|
[P2] Do not carry a selection into a different drawing by matching its ID
|
|
[P2] Compute a floating-pin input readout from the relative actuator angle
|
|
[P2] Key per-mechanism playback state by stable component identity
|
|
[P2] Revalidate the readiness rule for the active analysis mode
|
|
[P2] Open setup for the active analysis mode
|
|
[P2] Do not label a moving rocker as Grounded
|
|
[P2] Include synthesis work in the template replacement guard
|
|
[P2] Scope tutorial completion to the qualifying chain
|
|
[P2] Preserve meaningful
|
|
[P2] Record history only after an accepted synthesis edit
|
1 participant
What is in it
inject(), signals, ES2022, −3,300 lines of dead codeA drawing can hold several mechanisms
Everything used to go into one
Mechanism, so a second four-bar took the document to 2 degrees of freedom and both stopped simulating — the way to analyse one was to delete the other.The drawing now splits into connected components over moving links. The rule doing the work is that a grounded joint anchors what meets it but does not connect those things to each other: two cranks pinned to the same fixed point cannot feel each other and are two machines. Let ground connect and every grounded chain collapses back into one component, and the whole distinction disappears.
A component that reaches ground is a mechanism even when its mobility is wrong, because "this mechanism is 2-DoF" is a far more useful thing to tell a student than "you have no mechanism". Only a chain that never reaches ground is unassigned geometry.
Each machine gets its own row, its own direction, and its own reading of where it is in its cycle — M1 loops, M2 reverses. The one being analysed is drawn in full; the other is scenery.
Modes, and being told what is in the way
The old file toolbar and vertical rail took an edge and a corner between them. The modes take that space now and the file actions fold into one menu. Analyze splits into Kinematic and Force — different questions with different requirements, and one tab could only answer for both by being vague.
Pressing a mode that cannot be entered opens a list of what is in the way, per mechanism, each entry naming the way out. The old answer was one sentence about the whole document, first blocker wins.
The chip counts the fixes, the drawer explains each one, and the status strip along the bottom says the same thing in three words.
Synthesis
Place three positions of an end-effector link — by dropping them on the drawing or by typing X, Y and an angle — and the app enumerates every four-bar that admits them, ranks them, and previews one before anything touches the grid.
Each requirement narrows the search and says so. A solution reports the assembly branch it needs, which ground pin drives it best, and how far its crank actually rocks; Add driver sizes a crank and coupler so one full turn walks the linkage through all three, making it a six-bar a motor can run.
Export
Four steps — parts, kinematics, forces, file — instead of guessing from whatever happened to be selected. CSV, Excel, or a paginated printable report.
The right-click menu
Was one flat list of up to eight equally weighted rows with Delete at the top. Now a ladder — what you can attach, what the part is, then the destructive row last and alone — and a row that cannot be pressed says why rather than going quiet.
Editing
Framing
The canvas is full bleed and every panel floats over it, so "the window" and "the space you can see the linkage in" were never the same rectangle. Reset View framed to the window and centred the drawing partly behind the Edit panel.
Chrome now declares which edge it takes and the fit measures what is left:
Underneath: a view is either where a fit put it or where somebody drove it. A driven view is remembered and given back rather than taken away.
Deliberate departures from the mock
Compatibility
Every addition that travels in the URL writes nothing when it has nothing to say, so previously shared links and every template re-encode byte-identically.
J/L/Ftagged referencesCG/CJreferencesSD/SP/SR/SOentriesKJ/KFreferencesSelection left the URL entirely — the field is still written, always empty, to keep field positions load-bearing for existing links.
Verification
app.component.spec.tsagainst MATLAB results for a six-bardocs/fixture-urls.md), kept fresh by a spec that fails when it is staleBoth items previously listed as known, not done are now clean:
e2e/phase4-animation.mjspasses 8/8, andfull-tourreports zeroNG0100.