Sprint 4 [FIX] Frontend fixes: UN-2900, UN-3137, UN-3355, UN-3507 - #2258
Sprint 4 [FIX] Frontend fixes: UN-2900, UN-3137, UN-3355, UN-3507#2258hari-kuriakose wants to merge 7 commits into
Conversation
…ering Based on feat/shadcn-oss-migration (PR #2212), which rewrites both files. UN-3137: chunk_size is a token count -- it is passed straight to LlamaIndex's SentenceSplitter, documented as "the token chunk size for each chunk". The UI hint divided it by 4 (a characters-per-token estimate) and then by 1024, so a value of 1024 tokens was shown as "~= 0.3k tokens", under-reporting by roughly 4096x. Shows the token count directly, labels the field "Chunk Size (tokens)" and adds the unit to the profile info bar. UN-3355: highlight entries are [pageNumber, y, height, pageHeight]. Empty pages make LLMWhisperer emit a page number with y/height/pageHeight all zero. The filter kept those because `some(value => value !== 0)` is satisfied by the non-zero page number alone, so the viewer scrolled to the page and highlighted nothing. Requires usable geometry (finite values, positive height and page height) instead of any non-zero element. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF
Based on feat/shadcn-oss-migration (PR #2212). Index status was refreshed only when `indexDocs` changed, and that list is driven by websocket log messages. When those messages are dropped the backend still finishes indexing but the modal spins forever, because nothing ever re-queries the status. Adds the backend fallback the ticket asks for: while anything is indexing and the modal is open, poll the existing document-index endpoint every 5s and refresh both raw and summarize index status. The interval is cleared as soon as indexDocs empties or the modal closes, so there is no polling at rest. The websocket path is unchanged -- this only removes its status reporting as a single point of failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF
…variables Based on feat/shadcn-oss-migration (PR #2212). DEPENDS ON un-sprint4-D-backend (c830880): this reads `single_pass_unresolvable_variables`, which that branch adds to ToolStudioPromptSerializer. Until it lands the field is undefined and every guard here falls through to "no warning" -- degrades cleanly, but the feature does nothing on its own. Reuses the per-prompt notice pattern already in this header (Tooltip + Tag in the right-hand column, as used for progressMsg) rather than introducing a new surface. Tag color="warning" is a real preset in the shadcn shim's TAG_PRESET, and TriangleAlert is already imported from lucide elsewhere in this tree -- both checked, since the shims silently drop props they do not implement. Three delivery points, matching where the value can change: - page load: the tool fetch nests the field per prompt (backend branch). - prompt edit: handleChange folds single_pass_unresolvable_variables from the save response back into promptDetailsState. Only that field is taken, so the user's optimistic text is untouched. - single-pass toggled on: handleUpdateTool now folds a tool PATCH response back into the store when it carries prompts, so every affected prompt warns at once. Done here rather than in the toggle because SinglePassToggleSwitch is an enterprise plugin absent from OSS -- it only calls this shared function. The tooltip names the offending variables and says the text is sent to the LLM as-is; the tag stays terse ("N unresolved variables") to fit the header. NOT verified at runtime: this worktree has no node_modules, so esbuild parse is the ceiling -- no eslint, no render test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sXFBEu2GHatXq2CV2ShPF
#1 (High) ManageDocsModal: the 5s index-status poll could never stop on the path it was written for. `indexDocs` is emptied only by `deleteIndexDoc`, called from the websocket handlers -- so when socket messages are dropped it never empties and the interval ran forever, issuing two requests per tick and raising two failure toasts per tick. The poll now retires a document itself when the polled status reports it indexed, which stops the spinner and lets the effect tear its own interval down; poll-driven calls pass `silent` to suppress the repeating toast. User-initiated calls are unchanged. #2 (High) ToolIde: drop the `.then()` that merged `{...details, ...res.data}` into the store. `details` was a closure snapshot from the render that issued the PATCH, so a prompt added or deleted while the request was in flight was silently discarded. The single-pass toggle -- the only caller that changes single-pass mode, and so the only one UN-2900 needs refreshed prompts for -- already applies the response in its own `.then()`. #3, #4 (Medium) Correct two comments whose stated premises were false: the poll does not stop "as soon as indexDocs empties", and the single-pass toggle does not "only call this function" -- it consumes the response too. #5 (Low) ProfileInfoBar: chunk_size is nullable; render "-" instead of a dangling " tokens". 0 still renders "0 tokens". #6 (Low) AddLlmProfile: calcTokenSize no longer calculates anything -- rename to toTokenSize and drop the now-redundant `> 0` guard at the call site. Also clamps negatives, which the unguarded second call site previously displayed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE
…an id" Adversarial verification of the previous commit found the retirement predicate was wrong for the modal's primary action. `handleIsIndexed` answers "does this document have an index record", not "did the run now in flight finish". Re-indexing an already-indexed document leaves the old raw_index_id/summarize_index_id in place until the new run completes (prompt_studio_index_helper.handle_index_manager only ever writes the id, on completion, into a get_or_create'd row), so the first 5s tick retired the doc, cleared the spinner and re-enabled the buttons while Celery was still indexing. A first-ever index was unaffected, which is why the happy path looked correct. Take a fingerprint of each index row -- the id plus `modified_at`, which BaseModel bumps on every save -- when the poll arms, and retire a document only once that fingerprint actually changes. A row unseen at arm time is adopted rather than retired on first sighting, so a run starting mid-poll is not mistaken for a completed one. Also stop driving the loading indicator from polled calls: handleLoading toggles a SpinnerLoader in the Raw View / Summary View column headers, so leaving it on the 5s timer made those headers pulse for the whole indexing run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE
…onsumer path
A parallel review found that the rationale in the previous comment, while
true, was beside the point. PromptCard seeds promptDetailsState from its prop
exactly once and latches isPromptDetailsStateUpdated (PromptCard.jsx:74-83);
that flag is never reset -- setIsPromptDetailsStateUpdated(true) at :82 is its
only write outside the useState initializer -- and DocumentParser renders with
a stable key={item.prompt_id}, so nothing remounts to re-seed it. Header reads
the warning off promptDetailsState (PromptCard.jsx:370 -> Header.jsx:115), so
no store-level write to details.prompts can reach it by any path.
That makes omitting the write correct for a second, stronger reason than the
race, and it means "toggle single pass -> every affected prompt warns at once"
does not hold as shipped. Record both in the comment, along with the fact that
making it hold requires the consumer to accept updates after its first seed.
Per-prompt freshness on save is unaffected and continues to work via
PromptCard's own fold-back.
Comment-only; no behaviour change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NQhqNqCwFcXZZ7cQU6HxUE
…resolve-2258 # Conflicts: # frontend/src/components/custom-tools/prompt-card/Header.jsx
|
|
| Filename | Overview |
|---|---|
| frontend/src/components/custom-tools/manage-docs-modal/ManageDocsModal.jsx | Adds status polling, but its shared modified_at fingerprint can retire a document before all indexing stages finish. |
| frontend/src/components/custom-tools/pdf-viewer/PdfViewer.jsx | Tightens highlight filtering to require usable finite geometry; no concrete incompatible producer was established. |
| frontend/src/components/custom-tools/prompt-card/Header.jsx | Adds a guarded per-prompt warning for backend-reported unresolved single-pass variables. |
| frontend/src/components/custom-tools/prompt-card/PromptCard.jsx | Folds recomputed unresolved-variable metadata from prompt-save responses into local prompt state. |
| frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx | Corrects chunk-size presentation and conversion to consistently use token counts. |
| frontend/src/components/custom-tools/profile-info-bar/ProfileInfoBar.jsx | Labels profile chunk sizes in tokens and handles missing values. |
| frontend/src/components/custom-tools/tool-ide/ToolIde.jsx | Simplifies the tool PATCH promise path while documenting the warning-state synchronization boundary. |
Sequence Diagram
sequenceDiagram
participant UI as Manage Docs UI
participant API as Index API
participant IDX as Indexing pipeline
UI->>IDX: Start document reindex
UI->>UI: Add docId to indexDocs
UI->>API: Capture index-row baseline
IDX->>API: Update extraction/summary state and modified_at
UI->>API: Poll current index row
API-->>UI: Fingerprint changed
UI->>UI: deleteIndexDoc(docId)
Note over UI,IDX: UI unlocks while raw indexing may still run
IDX->>API: Finish raw index
Prompt To Fix All With AI
### Issue 1
frontend/src/components/custom-tools/manage-docs-modal/ManageDocsModal.jsx:419-425
**Fingerprint retires active indexing**
When a previously indexed document is reindexed without websocket updates, an intermediate extraction or summary update changes the shared `modified_at` fingerprint, so this branch removes the document from `indexDocs` before raw indexing finishes, stopping the spinner and re-enabling actions while indexing is still running.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/fea..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ( | ||
| baseline.get(key) !== fingerprint && | ||
| handleIsIndexed(indexType, item) && | ||
| indexDocs.includes(docId) | ||
| ) { | ||
| deleteIndexDoc(docId); | ||
| } |
There was a problem hiding this comment.
When a previously indexed document is reindexed without websocket updates, an intermediate extraction or summary update changes the shared modified_at fingerprint, so this branch removes the document from indexDocs before raw indexing finishes, stopping the spinner and re-enabling actions while indexing is still running.
Knowledge Base Used: Raise Gunicorn capacity for persistent WebSockets
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/components/custom-tools/manage-docs-modal/ManageDocsModal.jsx
Line: 419-425
Comment:
**Fingerprint retires active indexing**
When a previously indexed document is reindexed without websocket updates, an intermediate extraction or summary update changes the shared `modified_at` fingerprint, so this branch removes the document from `indexDocs` before raw indexing finishes, stopping the spinner and re-enabling actions while indexing is still running.
**Knowledge Base Used:** [Raise Gunicorn capacity for persistent WebSockets](https://app.greptile.com/zipstack/-/custom-context/knowledge-base/zipstack/unstract/-/reverts/incident-mitigation_2241-20260817-gunicorn-websocket-thread-pool-3be7483.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Confirmed real, and it turns out to be deeper than a frontend fix can reach. Tracked as UN-4056; not fixed in this PR, and I want to be explicit about why rather than leave it looking dismissed.
I attempted the obvious fix — key the fingerprint on the per-type index id plus index_ids_history depth instead of the shared modified_at — committed it, then had it adversarially verified. It was wrong twice over and I reverted it (ca78f0e40, reverted by 8c88f550d, whose message carries the full analysis).
It does not fix the defect. index_ids_history is also row-level. In prompt_studio_index_helper.py the index_id variable is per-type (:27-29), but the history append at :50-56 sits outside that branch — one shared list for both types. So a summarize completion moves the raw fingerprint, which is the same cross-talk as modified_at. Reachable via prompt_studio_helper.py:1514, which runs handle_index_manager(is_summary=True) before dynamic_indexer starts raw at :1530, with both on one row per :1441-1443.
And it introduces a hang. generate_index_key() is a deterministic content hash, so re-indexing an unchanged document yields the identical doc_id; the helper appends nothing (if doc_id not in index_ids) and rewrites the same id, so the key never moves. deleteIndexDoc is called only from the poll at :457, so on the socket-dead path this poll exists for, the spinner would spin forever. The current modified_at term does cover that case, since BaseModelQuerySet.update() bumps it.
The two failure modes are in tension: the row-level signal retires too early, the per-type signal never retires at all. There is no per-type "this run finished" marker on IndexManager for the frontend to observe — raw_index_id is stale-true on a re-index and everything else on the row is shared.
A sound fix needs a backend change (a per-type completion timestamp/status, or per-type history lists) or a poll timeout to bound the hang. Both are outside this PR's scope, so they're the decision recorded on UN-4056.
1 participant
Sprint 4 — frontend fixes
Important
Base is
feat/shadcn-oss-migration, notmain. Against the correct base this is 7 files / 3 commits. Againstmainit would show 300 files / 160 commits — almost all of them the shadcn migration itself. Please keep the base as set.8df78b403514226db105f0e2Dependency on the backend PR
The UN-2900 commit renders
single_pass_unresolvable_variables, which is produced by the backend branch (un-sprint4-D-backend).Either merge order is safe. Verified:
Header.jsxdefaults the value to[], the render guards on.length > 0, andPromptCardguards on!== undefined. Without the backend change this simply renders nothing — it is inert, not broken. An earlier note of mine claimed the order mattered; that was overstated and is corrected here.🤖 Generated with Claude Code
https://claude.ai/code/session_0197gRFp4QFxu2VzxhTEozGn