Enable in-place column sync, caching & reload

[cubap]

closes #346
Avoid full-page reloads after column operations by adding in-place column syncing, caching, and targeted refresh behavior. Adds cachedImageInfo/cachedAnnotations, methods to reload and refresh columns (reloadColumns, refreshFromCache), helpers to sync TPEN.activeProject from local state (syncProjectColumnsFromExisting, updateColumnsFromResponse, generateColumnId, getProjectPage, buildAssignedAnnotations), and local mutations for columns (create/extend/merge/clearColumnLocal). Column label handling now preserves rawLabel and maps autogenerated labels to "Unnamed" consistently. Server responses are parsed and used to update state when available; otherwise local fallbacks are applied. Also dispatches a "tpen-columns-updated" event and adds a listener in ColumnSelector to refresh only when updates target the current page.

[github-actions]

🚀 Deployed on https://69b02e9fe987e42d978c9627--tpen-interfaces-preview.netlify.app

[Copilot]

Pull request overview

This PR enables in-place column synchronization after column operations by replacing full-page reloads with targeted refresh behavior. It introduces caching mechanisms, event-driven updates, and local state mutations to keep the UI synchronized without interrupting the user experience.

Changes:

• Replaces window.location.reload() with reloadColumns() after column operations (merge, extend, create, clear)
• Adds caching of image info and annotations (cachedImageInfo, cachedAnnotations) for in-place re-rendering
• Implements column synchronization helpers to keep TPEN.activeProject aligned with local state
• Dispatches tpen-columns-updated events and adds a listener in ColumnSelector to refresh only when the current page is affected

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
interfaces/manage-columns/index.js	Adds caching, column reload logic, local mutation methods, and replaces full-page reloads with targeted refresh
components/create-column/index.js	Mirrors the same caching and reload changes as manage-columns for consistency
components/column-selector/index.js	Adds event listener for column updates and refreshes selector only when the current page is affected

Comments suppressed due to low confidence (10)

interfaces/manage-columns/index.js:444

• The parameter label is declared but never used in the function body. Consider removing it or clarifying its intended purpose if it should be incorporated into the generated ID.

    generateColumnId(label) {
        return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
    }

interfaces/manage-columns/index.js:705

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

                let data = null
                try { data = await res.json() } catch {}
                if (!this.updateColumnsFromResponse(data)) {
                    this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
                }

interfaces/manage-columns/index.js:807

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

                let data = null
                try { data = await res.json() } catch {}
                if (!this.updateColumnsFromResponse(data)) {
                    this.extendColumnLocal(columnToExtend, annotationIdsToAdd)
                }

interfaces/manage-columns/index.js:1030

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

            let data = null
            try { data = await res.json() } catch {}
            if (!this.updateColumnsFromResponse(data)) {
                this.createColumnLocal(columnLabel, selectedIDs)
            }

interfaces/manage-columns/index.js:1056

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

            let data = null
            try { data = await res.json() } catch {}
            if (!this.updateColumnsFromResponse(data)) {
                this.clearColumnsLocal()
            }

components/create-column/index.js:371

• The parameter label is declared but never used in the function body. Consider removing it or clarifying its intended purpose if it should be incorporated into the generated ID.

    generateColumnId(label) {
        return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
    }

components/create-column/index.js:631

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

                let data = null
                try { data = await res.json() } catch {}
                if (!this.updateColumnsFromResponse(data)) {
                    this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
                }

components/create-column/index.js:732

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

                let data = null
                try { data = await res.json() } catch {}
                if (!this.updateColumnsFromResponse(data)) {
                    this.extendColumnLocal(columnToExtend, annotationIdsToAdd)
                }

components/create-column/index.js:959

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

            let data = null
            try { data = await res.json() } catch {}
            if (!this.updateColumnsFromResponse(data)) {
                this.createColumnLocal(columnLabel, selectedIDs)
            }

components/create-column/index.js:985

• The empty catch block silently swallows parsing errors. Consider logging the error or providing a fallback message to aid debugging if JSON parsing fails unexpectedly.

            let data = null
            try { data = await res.json() } catch {}
            if (!this.updateColumnsFromResponse(data)) {
                this.clearColumnsLocal()
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubap]

@static-reviewer

[thehabes-static-reviewer]

Static Review Comments

Branch: 346-column-refresh-refactor
Review Date: 2026-03-05
Reviewer: Static Reviewer - @thehabes

Bryan and Claude make mistakes. Verify all issues and suggestions. Avoid unnecessary scope creep.

Category	Issues Found
🔴 Critical	1
🟠 Major	4
🟡 Minor	4
🔵 Suggestions	3

Critical Issues 🔴

Issue 1: Response body consumed twice — res.json() after success toast is read from an already-consumed stream

File: components/create-column/index.js:627-628 (and interfaces/manage-columns/index.js:701-702)
Category: Logic Error

Problem:
The response body is consumed by the success check (res.ok), and then res.json() is called inside a try/catch. However, the success toast is dispatched before res.json() is called. If the API returns a non-JSON success response (e.g., 204 No Content, or a plain text body), res.json() will throw, the empty catch silently swallows it, updateColumnsFromResponse(null) returns false, and the local fallback mutation runs — potentially creating state drift between client and server.

This pattern appears in all 4 operation handlers across both files (create, merge, extend, clear — 8 instances total).

Current Code:

TPEN.eventDispatcher.dispatch("tpen-toast", { 
    status: "success", message: 'Columns merged successfully.' 
})
let data = null
try { data = await res.json() } catch {}
if (!this.updateColumnsFromResponse(data)) {
    this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
}

Suggested Fix:

TPEN.eventDispatcher.dispatch("tpen-toast", { 
    status: "success", message: 'Columns merged successfully.' 
})
let data = null
try { data = await res.json() } catch (e) {
    console.warn('Could not parse column response as JSON:', e.message)
}
if (!this.updateColumnsFromResponse(data)) {
    this.mergeColumnsLocal(newLabel, columnLabelsToMerge)
}

At minimum, log a warning so developers know the fallback path was taken. The empty catch {} blocks make debugging impossible when the server response format changes or returns unexpected content types. Also consider checking res.headers.get('content-type') before attempting JSON parse, or checking res.status !== 204 first.

How to Verify:

1. Perform a column create/merge/extend/clear operation
2. Check the browser console — if the API does not return JSON, verify the warning appears and the local fallback correctly updates the UI
3. Verify there is no state mismatch between what's shown and what's on the server

---

Major Issues 🟠

Issue 1: extendColumnLocal receives display label instead of column index or raw label

File: components/create-column/index.js:731 and interfaces/manage-columns/index.js:806
Category: Logic Error

Problem:
When extendColumnLocal(columnToExtend, annotationIdsToAdd) is called as the local fallback, columnToExtend is an index (the numeric position from columnLabels.indexOf(label)). But extendColumnLocal() does a .findIndex() matching against col.label or col.rawLabel — it expects a label string, not an index.

When columnToExtend is a number like 0, the comparison col.label === 0 or col.rawLabel === 0 will never match a string, so targetIndex will be -1 and the function silently returns without extending anything.

Current Code:

// In the extend click handler:
columnToExtend = columnLabels.indexOf(label) !== -1 ? columnLabels.indexOf(label) : ''
// ...
if (!this.updateColumnsFromResponse(data)) {
    this.extendColumnLocal(columnToExtend, annotationIdsToAdd)
}

// In extendColumnLocal:
extendColumnLocal(columnLabel, annotationIdsToAdd) {
    const targetIndex = this.existingColumns.findIndex(col => col.label === columnLabel || col.rawLabel === columnLabel)

Suggested Fix:
Pass the actual label instead of the index:

if (!this.updateColumnsFromResponse(data)) {
    this.extendColumnLocal(this.originalColumnLabels[columnToExtend], annotationIdsToAdd)
}

Or alternatively, change extendColumnLocal to accept an index. But using the label is more consistent with how the API call uses this.originalColumnLabels[columnToExtend].

How to Verify:

1. Create multiple columns on a page
2. Extend a column when the API does NOT return a JSON body with columns
3. Verify the fallback correctly adds annotations to the right column

---

Issue 2: Massive code duplication between create-column and manage-columns

File: components/create-column/index.js and interfaces/manage-columns/index.js
Category: Code Quality / Maintainability

Problem:
The new methods added in this PR are identical across both files: reloadColumns(), getProjectPage(), generateColumnId(), updateColumnsFromResponse(), syncProjectColumnsFromExisting(), buildAssignedAnnotations(), refreshFromCache(), mergeColumnsLocal(), extendColumnLocal(), createColumnLocal(), clearColumnsLocal(). That's ~160 lines duplicated verbatim. The columnLabelCheck(), loadPage(), and the 4 operation handlers also share the same new patterns.

Any future bug fix or change will need to be applied in both files, making drift inevitable.

Suggested Fix:
Extract the shared column management logic into a shared mixin, utility module, or base class (e.g., utilities/columnOperations.js). Both components would import and delegate to the shared implementation. This doesn't need to block the PR but should be tracked as follow-up work.

How to Verify:
N/A — architectural concern. Verify by diff comparison that the implementations are indeed identical.

---

Issue 3: syncProjectColumnsFromExisting matches by label — unreliable for renamed or auto-labeled columns

File: components/create-column/index.js:388-409 and interfaces/manage-columns/index.js:461-482
Category: Logic Error

Problem:
syncProjectColumnsFromExisting() builds a Map keyed by column.label from the existing project page columns, then looks up matches using rawLabel. After columnLabelCheck() rewrites labels (auto-generated → "Unnamed N"), rawLabel preserves the original, but the byLabel map is built from page.columns which may hold either the raw server label or a previously-synced rawLabel. If two columns happen to share the same label (e.g., after a rename collision), they'll map to the same entry and one will be lost.

Additionally, the if (!updated.id && updated._id) updated.id = updated._id guard at line 407/480 is dead code — the preceding line id, already sets id from a chain that always produces a value (falling back to generateColumnId()), so updated.id can never be falsy.

Current Code:

const byLabel = new Map(existingColumns.map(column => [column.label, column]))
// ...
if (!updated.id && updated._id) updated.id = updated._id

Suggested Fix:
Key by column.id or column._id instead of label for reliable matching:

const byId = new Map(existingColumns.filter(c => c.id || c._id).map(c => [c.id ?? c._id, c]))
// then look up: const existing = byId.get(column.id) ?? byId.get(column._id)

Remove the dead if (!updated.id && updated._id) guard.

How to Verify:

1. Create two columns with similar labels
2. Perform a merge or rename operation
3. Verify the synced project state matches what the server returned

---

Issue 4: Unused label parameter in generateColumnId()

File: components/create-column/index.js:369 and interfaces/manage-columns/index.js:442
Category: Dead Code

Problem:
The label parameter is declared but never used inside generateColumnId(). This is misleading — callers pass a label expecting it to be incorporated into the ID generation, but it's silently ignored.

Current Code:

generateColumnId(label) {
    return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
}

Suggested Fix:
Either remove the parameter or use it:

generateColumnId() {
    return globalThis.crypto?.randomUUID?.() ?? `column-${Date.now()}-${Math.random().toString(16).slice(2)}`
}

How to Verify:
Search for all call sites of generateColumnId and confirm no caller depends on the label being part of the ID.

---

Minor Issues 🟡

Issue 1: cachedAnnotations initialized as empty array but checked with falsy guard

File: components/create-column/index.js:424 and interfaces/manage-columns/index.js:497
Category: Logic Error (minor)

Problem:
cachedAnnotations is initialized as [] in the constructor (line 35). In refreshFromCache(), the guard !this.cachedAnnotations will never be true for an empty array (since [] is truthy). This means if loadPage() hasn't been called yet but cachedAnnotations remains [], the method will proceed with empty data rather than returning false.

Current Code:

refreshFromCache() {
    if (!this.cachedImageInfo || !this.cachedAnnotations) return false

Suggested Fix:

refreshFromCache() {
    if (!this.cachedImageInfo || !this.cachedAnnotations?.length) return false

How to Verify:

1. Trigger reloadColumns() before loadPage() has completed
2. Verify the component correctly falls back to the warning toast instead of rendering empty annotations

---

Issue 2: columnLabelCheck() is declared async but contains no await

File: components/create-column/index.js:285 and interfaces/manage-columns/index.js:355
Category: Code Hygiene

Problem:
columnLabelCheck() is declared as async but performs only synchronous operations. This wraps the return value in an unnecessary Promise and callers await it for no reason.

Current Code:

async columnLabelCheck() {
    this.originalColumnLabels = this.existingColumns.map(col => col.rawLabel ?? col.label)
    // ...purely synchronous...
}

Suggested Fix:

columnLabelCheck() {
    this.originalColumnLabels = this.existingColumns.map(col => col.rawLabel ?? col.label)
    // ...
}

How to Verify:
Remove async, verify no call site depends on it returning a Promise.

---

Issue 3: Column label duplicate check compares display labels — may miss collision with raw labels

File: components/create-column/index.js:601-603 and interfaces/manage-columns/index.js:675-677
Category: Logic Error (minor)

Problem:
When checking for duplicate labels before merge, the code compares against col.label (the display label, e.g., "Unnamed 1"). If a user enters "Unnamed 1" as their new merge label, the duplicate check catches it. But if they enter the raw auto-generated label (e.g., "Column 6789..."), the check won't flag it as duplicate even though it would collide server-side.

Current Code:

const duplicate = this.existingColumns.some(col => {
    const existingLabel = (col.label ?? "").toString().trim()
    return existingLabel === newLabel
})

Suggested Fix:
Check against both label and rawLabel:

const duplicate = this.existingColumns.some(col => {
    const displayLabel = (col.label ?? "").toString().trim()
    const rawLabel = (col.rawLabel ?? "").toString().trim()
    return displayLabel === newLabel || rawLabel === newLabel
})

How to Verify:

1. Have a column with an auto-generated label like "Column 6789abcdef..."
2. Try to create/merge a new column with that exact raw label string
3. Verify the duplicate check fires

---

Issue 4: ColumnSelector.refreshFromProject() duplicates logic from findColumnsData()

File: components/column-selector/index.js:91-119
Category: Code Hygiene

Problem:
refreshFromProject() duplicates the page-finding, column-mapping, and label-checking logic from findColumnsData() (lines 65-88). The only difference is that refreshFromProject() skips the async vault.getWithFallback() call when this.#page is already loaded. This duplication makes maintenance harder.

Suggested Fix:
Consider refactoring findColumnsData() to accept a skipFetch parameter, or extract the shared column-mapping logic into a private helper.

How to Verify:
Compare the two methods side by side to confirm the shared logic is identical.

---

Suggestions 🔵

Suggestion 1: Consider using Array.prototype.some() instead of Array.prototype.find() for filtering

File: components/create-column/index.js:428 and interfaces/manage-columns/index.js:501

The .filter(anno => !assignedAnnotationIds.find(...)) pattern in refreshFromCache() and loadPage() uses .find() which returns the matched element. Since only a boolean is needed, .some() is semantically clearer and can short-circuit earlier:

.filter(anno => !assignedAnnotationIds.some(a => a.lineId === anno.lineId))

---

Suggestion 2: Workspace toolbar re-render after reloadColumns() may leave stale merge/extend UI

File: components/create-column/index.js:337-361 and interfaces/manage-columns/index.js:410-434

After reloadColumns() completes, the merge/extend workspace buttons still reference the old column labels (the closure captures columnLabelsToMerge and columnLabels from before the reload). If a user tries to merge again without toggling the mode off/on, the workspace UI will be out of sync with the new column state. Consider re-triggering handleModeChange() or resetting the mode checkboxes after reloadColumns().

---

Suggestion 3: mergeColumnsLocal type mismatch risk with filter comparison

File: components/create-column/index.js:441-447 and interfaces/manage-columns/index.js:513-520

mergeIndexes comes from columnLabelsToMerge which stores numeric indexes (from columnLabels.indexOf()). The .filter((_, index) => !mergeIndexes.includes(index)) comparison is safe as long as both sides are numbers. However, in create-column/index.js, the merge click handler pushes the result of columnLabels.indexOf(label) !== -1 ? columnLabels.indexOf(label) : '' — the fallback '' (empty string) is then filtered out by .filter(i => i !== ''), but 0 (a valid index) would also be filtered if using a loose check. Currently it's strict !== so index 0 is safe, but this is fragile. Consider using -1 as the sentinel instead of ''.

---

Files Reviewed

• components/column-selector/index.js - New event listener for column updates and refreshFromProject() method
• components/create-column/index.js - Caching, local mutation, and in-place reload added to replace window.location.reload()
• interfaces/manage-columns/index.js - Mirror of create-column changes for the interface version

If there are significant code changes in response to this review please test those changes. Run the application manually and test or perform internal application tests when applicable.

[cubap]

If copilot is happy with this, so am I

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[thehabes]

Manual Testing Comments

1. Clear All Columns shows error toast and success toast. It seems to succeed, but the interface does not update. This is Major Issue 1 from the static review.

2. Once I select 'Extend Column Mode' I cannot select lines. Even after I unselect it. This is a pre-existing issue not introduced by this PR (it currently happens on main as well).

---

Static Review Comments

Category	Issues Found
🔴 Critical	0
🟠 Major	1
🟡 Minor	3
🔵 Suggestions	1

Critical Issues 🔴

None.

---

Major Issues 🟠

Issue 1: res.json() called after res.ok check but response may not have JSON body

File: components/create-column/index.js:626,729,958,983 and interfaces/manage-columns/index.js:700,804,1029,1054
Category: Unhandled Error

Problem:
After a successful API response, the code calls const data = await res.json() to extract column data from the response. If the server returns a 204 No Content, an empty body, or non-JSON body, res.json() will throw, and the catch block will show a generic error toast — even though the operation actually succeeded (the success toast was already dispatched).

This is most likely to happen with the DELETE /clear-columns endpoint, which may return an empty body on success.

Current Code (e.g., clearAllSelections):

if (!res.ok) throw new Error(`Server error: ${res.status}`)
TPEN.eventDispatcher.dispatch("tpen-toast", {
    status: "info", message: 'All columns cleared successfully.'
})
const data = await res.json()  // Could throw on empty body
if (!this.updateColumnsFromResponse(data)) {
    this.clearColumnsLocal()
}
await this.reloadColumns()

Suggested Fix:

if (!res.ok) throw new Error(`Server error: ${res.status}`)
TPEN.eventDispatcher.dispatch("tpen-toast", {
    status: "info", message: 'All columns cleared successfully.'
})
let data = null
try { data = await res.json() } catch { /* empty response body */ }
if (!data || !this.updateColumnsFromResponse(data)) {
    this.clearColumnsLocal()
}
await this.reloadColumns()

How to Verify:

1. Clear all columns on a page
2. Check network tab — if server returns empty body, verify no error toast appears after the success toast

---

Minor Issues 🟡

Issue 2: extendColumn() in create-column still uses indexOf(label) — wrong index for duplicate labels

File: components/create-column/index.js:677
Category: Logic Error

Problem:
The mergeColumns() method was correctly updated to use the labelIndex parameter from forEach, but extendColumn() in the same file was not. It still uses columnLabels.indexOf(label), which returns the first occurrence. If two columns have the same display label (e.g., two "Unnamed 1" entries after a label collision), indexOf will always resolve to the first match, making it impossible to select the second one.

The manage-columns/index.js version of extendColumn() (line 752) was correctly updated to use labelIndex. This is an inconsistency between the two files.

Issue 3: createColumn() duplicate check doesn't compare rawLabel — inconsistent with mergeColumns()

File: components/create-column/index.js:929-932 and interfaces/manage-columns/index.js:1000-1003
Category: Logic Error / Inconsistency

Problem:
The mergeColumns() duplicate check was updated to compare against both col.label (display label) and col.rawLabel (server label). However, createColumn() in both files still only checks against col.label. This means a user could create a column with a label that matches an existing column's raw/server label (e.g., "Column 65f1a2b3c4d5e6f7a8b9c0d1"), which would likely cause a server-side conflict or unexpected behavior.

Current Code (both files):

const duplicate = this.existingColumns.some(col => {
    const existingLabel = (col.label ?? "").toString().trim()
    return existingLabel === columnLabel
})

Suggested Fix:

const duplicate = this.existingColumns.some(col => {
    const displayLabel = (col.label ?? "").toString().trim()
    const rawLabel = (col.rawLabel ?? "").toString().trim()
    return displayLabel === columnLabel || rawLabel === columnLabel
})

How to Verify:

1. Have a column with an auto-generated label (displays as "Unnamed 1" but raw is "Column abc123...")
2. Try creating a new column with the raw label string as the title
3. Verify the duplicate check catches it

Current Code:

// components/create-column/index.js:662
columnLabels.forEach(label => {
    // ...
    this.workspaceCleanup.onElement(btn, 'click', () => {
        // ...
        columnToExtend = columnLabels.indexOf(label) !== -1 ? columnLabels.indexOf(label) : ''

Suggested Fix:

// components/create-column/index.js:662
columnLabels.forEach((label, labelIndex) => {
    // ...
    this.workspaceCleanup.onElement(btn, 'click', () => {
        // ...
        columnToExtend = labelIndex

Issue 4: generateColumnId() accepts a parameter that's never used

File: components/create-column/index.js:369 and interfaces/manage-columns/index.js:442
Category: Dead Code / Code Hygiene

Problem:
The method signature is generateColumnId() with no parameters, but it's called with arguments in several places: this.generateColumnId(rawLabel), this.generateColumnId(newLabel). The passed argument is silently ignored. This isn't a bug (the function works), but it's misleading — a reader might think the label is used for ID generation.

---

Suggestions 🔵

Suggestion 1: Significant code duplication between create-column and manage-columns

Category: Code Quality / Maintainability

The following methods are nearly identical across both files (~160 lines duplicated):

• reloadColumns()
• getProjectPage()
• generateColumnId()
• updateColumnsFromResponse()
• syncProjectColumnsFromExisting()
• buildAssignedAnnotations()
• refreshFromCache()
• mergeColumnsLocal()
• extendColumnLocal()
• createColumnLocal()
• clearColumnsLocal()
• columnLabelCheck()

Consider extracting these into a shared mixin or utility module (e.g., utilities/columnManager.js) that both components can import. This would make future bug fixes and feature changes a single-point edit instead of requiring synchronized changes across two files. This is a follow-up concern — not necessarily something for this PR.

If there are significant code changes in response to this review please test those changes. Run the application manually and test or perform internal application tests when applicable.

[cubap]

Just trouble with the return type, so I over-engineered the response. It may become a helper util later.

[thehabes]

A couple of quirks that have their own issue, but otherwise functioning as intended.