Repository navigation
OpenConceptLab/ocl_issues#2830 | stop Auto Match when leaving the project - #114
Conversation
…ject
- Ask before leaving while a run is going ("Stay" / "Stop and leave"); the
browser's prompt covers closing the tab.
- Leaving stops the run at once: its record closes as stopped, late results
are dropped, and no autosave is scheduled after unmount.
- Closing the tab closes the run record with a keepalive request.
- Fix isRunningBulkScispacyCandidates checking the start time for its end.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
snyaggarwal
left a comment
There was a problem hiding this comment.
Found two leave paths that can still leave an Auto Match run active. The new helper tests pass (9/9), but they do not exercise these lifecycle transitions.
…ave, also when the page is reused
8cc3a89 to
fa48989
Compare
|
Addressed the review (
The leave lifecycle in 🤖 Generated with Claude Code |
snyaggarwal
left a comment
There was a problem hiding this comment.
Re-review of fa48989: the run now stops before navigation, and the companion API PR covers a run whose create response is lost on tab close. Two navigation cases still need attention.
…, and keep a save from pulling the user back
|
Addressed the re-review (
🤖 Generated with Claude Code |
snyaggarwal
left a comment
There was a problem hiding this comment.
Re-review of 3feee85: the previous two navigation findings are addressed. I found one remaining history-navigation bug in the new page-key logic. The 12 helper tests pass, but they omit the direct Back/Forward sequence below.
…eping it only for a save's own navigation
|
Addressed the re-review (
🤖 Generated with Claude Code |
snyaggarwal
left a comment
There was a problem hiding this comment.
Re-reviewed 1120fa0. The direct new-project → saved-project → Back → Forward sequence now remounts on both history moves; the save's immediate push retains the instance as intended. The 13 focused helper tests pass. I found no further actionable issue in this revision. The MapProject leave lifecycle still has no browser/API integration test.
|
Codex review, pass 1 (codex-cli 0.164.0, commit 1120fa0) An independent pass for my review, run on top of Sunny's four rounds. The scenarios are in the JSON below; Codex says it reproduced each one against the real helpers. I haven't checked them by hand. F1.1 and F1.2 bear on the ticket's second criterion, that nothing spends quota after the user leaves. Verdict: findings. 5 P1, 1 P2.
findings.json{
"reviewer": "codex-cli 0.164.0",
"perspective": "defects",
"mode": "full",
"commit": "1120fa0",
"verdict": "findings",
"prior": [],
"findings": [
{
"id": "F1.1",
"severity": "P1",
"tag": null,
"file": "src/components/map-projects/MapProject.jsx",
"line": 402,
"claim": "Leaving can still trigger new rerank requests after the run stops.",
"scenario": "Stop and leave a multi-algorithm run while candidate lookups are pending. The run's finally clears isBulkMatchRunningRef and bulkMatchAlgoIdsRef. A late lookup then invokes scheduleRerank, which classifies the work as manual and passes isRunTraffic=false. Consequently rerank ignores abortRef and sends new $rerank requests after unmount. This was reproduced using the actual scheduler and rerank functions.",
"action": "Gate rerank scheduling and sending on the component's lifetime, and prevent late run callbacks from becoming manual traffic after cancellation.",
"lines_delta": null,
"confidence": "high"
},
{
"id": "F1.2",
"severity": "P1",
"tag": null,
"file": "src/components/map-projects/MapProject.jsx",
"line": 402,
"claim": "Setting abortRef does not cancel an AI retry already sleeping.",
"scenario": "An AI invocation fails to connect and enters retryWithBackoff's delay. The user chooses Stop and leave during that delay. retryWithBackoff checks cancellation before sleeping but not before its next attempt, so another invocation is sent after leaving and can consume an AI call. A direct reproduction confirms the second attempt runs with cancellation already true.",
"action": "Check cancellation before every retry attempt, including immediately after the backoff delay, and add a regression test for leaving during that delay.",
"lines_delta": null,
"confidence": "high"
},
{
"id": "F1.3",
"severity": "P1",
"tag": null,
"file": "src/components/map-projects/MapProject.jsx",
"line": 393,
"claim": "A delayed leave save still combines frozen React state with post-unmount refs.",
"scenario": "A project save is uploading when the user stops and leaves an active run. saveOnLeave delays onSave until that upload settles. Meanwhile outstanding match or AI responses still update candidate, stage, and log refs because leaving does not invalidate their run ticket. React state updates after unmount are discarded. The delayed save therefore includes new candidates or completed stages alongside old mappings or missing analysis\u2014the inconsistent snapshot described by the ticket.",
"action": "Capture a complete immutable save payload before unmount, or prevent all late run responses from mutating the departing project's refs. Test with a pending save and a response arriving after unmount.",
"lines_delta": null,
"confidence": "high"
},
{
"id": "F1.4",
"severity": "P1",
"tag": null,
"file": "src/components/map-projects/MapProject.jsx",
"line": 2947,
"claim": "Reopening the project can overtake the newly queued stop-and-leave save.",
"scenario": "Save A is pending when Stop and leave queues save B containing the run's results. Reopen the project before A finishes: whenSaved captures only A. Once A settles, B starts, but the reopening GET also starts without waiting for B. It can load A's older contents, and a subsequent autosave can overwrite B's results. This ordering reproduces with the existing save helpers.",
"action": "Register the entire queued save chain synchronously and make whenSaved wait through its completion. Return the request from onSave so queued saves can be tracked correctly.",
"lines_delta": null,
"confidence": "high"
},
{
"id": "F1.5",
"severity": "P1",
"tag": null,
"file": "src/components/map-projects/MapProject.jsx",
"line": 404,
"claim": "Stop and leave discards an unsaved project's results despite promising they are saved.",
"scenario": "On /map-projects/new, upload a file, configure matching, and run Auto Match before the first Save; the UI permits this. The leave dialog says matched results are saved, but the scheduler ignores projects without an ID and automatic onSave also returns without one. Choosing Stop and leave therefore discards the project and all results without saving anything.",
"action": "Persist a valid new project before leaving, or explicitly warn that the unsaved project and results will be discarded instead of promising a save.",
"lines_delta": null,
"confidence": "high"
},
{
"id": "F1.6",
"severity": "P2",
"tag": null,
"file": "src/components/map-projects/leaveGuard.js",
"line": 15,
"claim": "Replaying a blocked POP as PUSH defeats the page-remount safeguard.",
"scenario": "Save project A from /map-projects/new, go Back, and start matching another unsaved project. Press Forward to A and choose Stop and leave. continueLeaving pushes A's stored location, retaining mapProjectFrom='/map-projects/new'. getPageKey now sees PUSH and preserves the current key, so A's URL displays the unrelated unsaved project and never loads A.",
"action": "Remove the save-navigation marker when replaying user navigation, or use a one-time marker that distinguishes an actual save from a replayed history entry. Test continueLeaving and getPageKey together.",
"lines_delta": null,
"confidence": "high"
}
]
} |
…e leave save at once, and warn when an unsaved project is discarded
|
Addressed the Codex review, pass 1 (
🤖 Generated with Claude Code |
Fixes OpenConceptLab/ocl_issues#2830, together with OpenConceptLab/oclapi2#960: leaving a project while Auto Match runs no longer leaves the run going.
Changes
auto_match_stopped_on_leave, closes the run record as stopped at once, and saves what was matched so far, before navigating. This doesn't depend on the page unmounting:/map-projects/newand project URLs share oneMapProjectinstance. Unmounting runs the same stop if the dialog didn't. Each run record is closed only once (createRunClosers).MapProjectloads its project only on mount, so the routes now render it throughMapProjectRoute, which remounts it for a different project URL. The page's own redirect after a save (e.g./map-projects/new→ the saved project) passesmapProjectFromin the location state and keeps the instance. Only that push or replace keeps it: Back and Forward replay the state, so they always get a new page. When the dialog continues a held Back or Forward as a push, it drops that state. Pages are keyed by a counter, so a new page never reuses an old key.retryWithBackoffchecks for cancellation after its wait too, so an AI call waiting to retry isn't sent again.onSavetakesafter, a save still in flight. The leave save builds its payload when the user leaves and sends it once that save settles. Late results can't get into it, and it's tracked right away, so reopening the project waits for it (whenSaved).scheduleAutoSavedoes nothing once the page is gone, so late results can't save state frozen at unmount together with live refs.pagehide, the run is stopped and its record closed via a newAPIService.keepalive()(fetch withkeepalive, which outlives the page; axios's XHR doesn't). A close already in flight is resent with its original status.isRunningBulkScispacyCandidateschecked the start time where it should check the end time.Notes
runningwith no update for 24 hours.Testing
npm test: 596 pass, including newleaveGuard.test.js.MapProjecthas no automated test.🤖 Generated with Claude Code