Split /progress/<token> into lightweight polling and heavyweight full-data endpoints - #72
Conversation
- `/progress/<token>` now returns only scalar fields (status, current, total, step, error, error_code, _live) - no chapter content or reports - New `/progress/<token>/full` endpoint returns the complete payload - Frontend fetchFullProgress() fires every 2 minutes or on chapter completion; on generation done it always fetches full data first - Added error callback to fetchFullProgress for resilience - New tests verify the lightweight/full split and 404 behaviour Agent-Logs-Url: https://github.com/CyberSecDef/NovelForge/sessions/5134a394-39a0-40dd-b06d-0a98120d334b Co-authored-by: CyberSecDef <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR splits chapter-generation progress polling into a lightweight endpoint for frequent polling and a new heavyweight endpoint for fetching full progress data (chapters + reports), reducing payload size and client/server load during generation.
Changes:
- Backend:
GET /progress/<token>now returns only lightweight scalar fields; newGET /progress/<token>/fullreturns the complete progress payload. - Frontend: polling uses the lightweight endpoint and periodically/conditionally fetches full data via a new helper.
- Tests: adds integration coverage for lightweight vs full responses and unknown-token 404s; reduces test duplication via a thread-patching helper.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
novelforge/routes/generation.py |
Implements lightweight /progress/<token> and new /progress/<token>/full endpoint. |
static/js/script.js |
Updates polling to use lightweight endpoint and adds full-progress fetch logic on triggers. |
tests/test_integration.py |
Adds integration tests to verify lightweight/full endpoint behavior and unknown-token handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with _progress_lock: | ||
| data = _progress_store.get(token) | ||
| if data is None: | ||
| return jsonify({"error": "Unknown token"}), 404 | ||
| light = {k: data[k] for k in _LIGHTWEIGHT_FIELDS if k in data} |
There was a problem hiding this comment.
data is retrieved under _progress_lock, but the lightweight snapshot is built after the lock is released. Because the background generator mutates the per-token dict under the same lock, building light outside the lock can race with those mutations (and can raise RuntimeError: dictionary changed size during iteration or return an inconsistent mix of fields). Build the lightweight dict while still holding _progress_lock (and ideally copy out only the needed scalar values) before releasing the lock.
| with _progress_lock: | ||
| data = _progress_store.get(token) | ||
| if data is None: | ||
| return jsonify({"error": "Unknown token"}), 404 | ||
| return jsonify(data) | ||
| return jsonify(dict(data)) |
There was a problem hiding this comment.
dict(data) is created after _progress_lock is released. Since the background generation thread mutates the same dict under the lock, iterating/copying it outside the lock can race and throw RuntimeError: dictionary changed size during iteration, causing intermittent 500s on /progress/<token>/full. Create the snapshot copy while holding _progress_lock (e.g., copy to a local variable inside the with block) and then jsonify the snapshot after releasing the lock.
| * Updates _latestFullData, refreshes the chapter list, and calls | ||
| * showDoneStep if generation has finished. |
There was a problem hiding this comment.
The docblock for fetchFullProgress() says it "calls showDoneStep if generation has finished", but the function only refreshes _latestFullData and the chapter list; it never checks status or calls showDoneStep. Please update/remove this part of the comment so future changes don’t rely on incorrect behavior.
| * Updates _latestFullData, refreshes the chapter list, and calls | |
| * showDoneStep if generation has finished. | |
| * Updates _latestFullData, refreshes the chapter list, and invokes | |
| * the optional onComplete callback with the latest full payload. |
| if (data.status === "done") { | ||
| showDoneStep(data); | ||
| // Generation complete – fetch full payload before showing results | ||
| fetchFullProgress(function (fullData) { | ||
| showDoneStep(fullData); | ||
| }); |
There was a problem hiding this comment.
When data.status === "done", the UI now always calls fetchFullProgress() and passes its result to showDoneStep(). If the /full request fails, fetchFullProgress invokes the callback with _latestFullData (often {}), which makes showDoneStep render an empty "done" state (no chapters/consistency) even though generation finished. For the done path, treat the full fetch as required: retry, show an error and keep polling/fetching until full data arrives, or at least fall back to the last known good full payload and only call showDoneStep when it contains the expected fields (e.g., chapters_done).
Every poll to
/progress/<token>returned the full progress object — growing unboundedly with manuscript size as chapters and 9 post-generation reports accumulated. Polling clients paid full payload cost on every tick even when they only neededstatus/step.Changes
Backend —
novelforge/routes/generation.pyGET /progress/<token>now returns only 7 scalar fields:status,current,total,step,error,error_code,_liveGET /progress/<token>/full(new) returns the complete payload:chapters_done,consistency, all 9 audit reports,character_state_log, etc.Frontend —
static/js/script.jspollProgress()uses only the lightweight endpoint in its adaptive 15–60s loopfetchFullProgress()helper calls/progress/<token>/fullwith both success and error callbacks (failures are non-fatal; lightweight polling is unaffected)currentincrements_fullFetchIntervalMs = 120000)showDoneStepTests —
tests/test_integration.pytest_progress_endpoint_is_lightweight— assertschapters_done/consistencyabsent from lightweight responsetest_progress_full_endpoint_includes_heavy_fields— asserts complete data present on/fulltest_progress_endpoint_unknown_token/test_progress_full_endpoint_unknown_token— 404 coverage_patch_thread()helper to remove duplication across test methodsWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
api.openai.com/usr/bin/python python -m pytest tests/ -v(dns block)If you need me to access, download, or install something from one of these locations, you can either: