gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-09-24 11:26:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(tags): tag chip editor on the item detail page (TASK-1654) (#659)

Browse Source 
* feat(tags): tag chip editor on the item detail page (TASK-1654)

Tags live on item.tags (a JSON-array string), not the collection schema, so
this adds a TagInput sibling to FieldEditor rather than a field type.

- TagInput.svelte: chip editor — Enter/comma to add, Backspace/× to remove,
  case-insensitive dedupe (stored as typed), autocomplete dropdown sourced
  from the workspace tag set; readonly mode renders plain chips.
- Item detail page: derive `tags` from item.tags (defensive parse),
  load `tagSuggestions` via a workspace-keyed $effect kept separate from the
  item-load path (Svelte 5 effect-splitting convention), and updateTags()
  mirrors updateField() — optimistic with revert-on-failure, PATCHing `tags`.
  The Tags row renders between the schema fields and the Assignment section.

api.items.update already accepted `tags` via ItemUpdate, so no client change
was needed there.

Parent: PLAN-1652.

* fix(tags): guard overlapping tag saves with a sequence counter per Codex review (round 1)

Rapid chip edits can issue overlapping PATCHes; a late-resolving older
request could clobber the newer tag set with stale data or an errant revert.
Only the latest save (by monotonic seq) applies its result or reverts.

* fix(tags): drop stale tag-suggestion results across workspace navigation per Codex review (round 2)

loadTagSuggestions now only assigns when the in-flight workspace still
matches the current one, so a slower old-workspace /tags response can't
overwrite the new workspace's autocomplete.

* fix(tags): dedupe tags + key chips by index per Codex review (round 3)

An item can carry duplicate tags (e.g. ["ux","ux"]) since the write path
doesn't enforce per-item uniqueness, which would collide value-based Svelte
keys. Key chips by index, and dedupe case-insensitively at the source so the
cleaned set persists on the next save.

* fix(tags): gate tag-save completion UI on item freshness per Codex review (round 4)

If the user navigates to another item while a tag save is in flight (no
further edit, so the seq guard doesn't trip), skip showSaved()/toast/refresh
so completion UI can't fire on an unrelated page.

* fix(tags): serialize+coalesce tag saves, revert to last confirmed per Codex review (round 5)

Replace the concurrent-PATCH-with-seq-guard approach with a single
in-flight, coalescing saver scoped per item. Eliminates the overlap class
structurally: no stale completion clobbers a newer set, and `confirmed`
tracks the last server-acknowledged tags so a failed save reverts to server
truth rather than an optimistic unconfirmed value. Subsumes the round-1 race
guard and round-4 navigation gate.

* fix(tags): key tag savers by item id to prevent cross-navigation concurrency per Codex review (round 6)

A single saver slot let navigating away from an item mid-save and back spawn
a second concurrent saver for it. Hold savers in a Map keyed by item id so
edits coalesce into the existing in-flight saver; evict on drain.

* fix(tags): reapply in-flight desired tags after item reload per Codex review (round 7)

Navigating away and back mid-save reloaded stale server tags; a follow-up
edit computed from that stale set could drop the in-flight edit. The saver
now tracks the latest desired set and loadData reapplies it when a save is
still in flight for the reloaded item.

* fix(tags): keep save indicator active across reload so refresh guards hold per Codex review (round 8)

loadData reset saveStatus to idle while a tag PATCH was still in flight,
letting SSE/sync snapshot adoption bypass the saveStatus==='saving' guard and
land stale tags. Restore 'saving' when reapplying an in-flight saver so the
existing refresh guards keep skipping until the save drains.

* fix(tags): overlay in-flight tags at every server-snapshot assignment per Codex review (round 9)

The saveStatus guard is racy (checked before the refresh handlers' own await,
not after), so a concurrent snapshot could still drop optimistic tags. Extract
withInflightTags() and route ALL item = <server snapshot> sites through it
(realtime SSE/sync, initial load, content-save echoes, title/field/assignment/
role update echoes, post-action refresh, version restore). Overlaying the
saver's desired set at assignment time is race-free regardless of the guard.

* fix(tags): overlay tags on field-save + forced-retry echoes per Codex review (round 10)

updateField's success echo (item = fresh) and the forced open-children retry
(item = forced) were the last two un-overlaid server-snapshot assignments;
route both through withInflightTags so a concurrent tag save isn't clobbered.

* fix(tags): preserve unsaved content when reconciling tag-save echo per Codex review (round 11)

flushTagSaver adopted the full tag PATCH response (item = fresh), which
carries server content and could clobber unsaved editor edits. Route it
through adoptServerItem so local content is preserved (non-collab) like the
other snapshot adoption sites.
This commit is contained in:
xarmian
2026-05-30 01:58:17 -04:00
committed by GitHub
parent 1b1068537c
commit feb068a91f
2 changed files with 401 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/components/fields/TagInput.svelte
+199
View File
@@ -0,0 +1,199 @@
<script lang="ts">
/**
* Free-form tag chip editor. Tags live on `item.tags` (a JSON-array
* string), NOT in the collection schema — so this is a sibling of
* FieldEditor rather than a field type. Emits the full new tag array via
* `onchange`; the parent persists it (PATCH items.tags). Dedupe is
* case-insensitive but tags are stored as typed (human-readable).
*/
interface Props {
tags: string[];
onchange: (tags: string[]) => void;
suggestions?: string[];
readonly?: boolean;
}
let { tags, onchange, suggestions = [], readonly = false }: Props = $props();
let inputValue = $state('');
let showSuggestions = $state(false);
function hasTag(value: string): boolean {
const v = value.trim().toLowerCase();
return tags.some((t) => t.toLowerCase() === v);
}
function addTag(raw: string) {
const value = raw.trim();
inputValue = '';
showSuggestions = false;
if (!value || hasTag(value)) return;
onchange([...tags, value]);
}
function removeTag(index: number) {
onchange(tags.filter((_, i) => i !== index));
}
function handleKeydown(e: KeyboardEvent) {
if (e.key === 'Enter' || e.key === ',') {
e.preventDefault();
addTag(inputValue);
} else if (e.key === 'Backspace' && inputValue === '' && tags.length > 0) {
removeTag(tags.length - 1);
} else if (e.key === 'Escape') {
showSuggestions = false;
}
}
// Suggestions not already applied, matched case-insensitively to the input.
let filteredSuggestions = $derived.by(() => {
const q = inputValue.trim().toLowerCase();
return suggestions
.filter((s) => !hasTag(s))
.filter((s) => q === '' || s.toLowerCase().includes(q))
.slice(0, 8);
});
</script>
{#if readonly}
<div class="tag-chips">
{#if tags.length === 0}
<span class="tag-empty">No tags</span>
{:else}
<!-- Key by index: the write path doesn't enforce per-item tag
uniqueness, so a value key would collide on ["ux","ux"]. -->
{#each tags as tag, i (i)}
<span class="tag-chip readonly">{tag}</span>
{/each}
{/if}
</div>
{:else}
<div class="tag-input">
<div class="tag-chips">
{#each tags as tag, i (i)}
<span class="tag-chip">
{tag}
<button
type="button"
class="tag-remove"
aria-label={`Remove ${tag}`}
onclick={() => removeTag(i)}>×</button
>
</span>
{/each}
<input
bind:value={inputValue}
class="tag-entry"
type="text"
placeholder={tags.length === 0 ? 'Add tags…' : ''}
onkeydown={handleKeydown}
onfocus={() => (showSuggestions = true)}
onblur={() => setTimeout(() => (showSuggestions = false), 120)}
/>
</div>
{#if showSuggestions && filteredSuggestions.length > 0}
<div class="tag-suggestions">
{#each filteredSuggestions as s (s)}
<button
type="button"
class="tag-suggestion"
onmousedown={(e) => {
e.preventDefault();
addTag(s);
}}>{s}</button
>
{/each}
</div>
{/if}
</div>
{/if}
<style>
.tag-input {
position: relative;
width: 100%;
}
.tag-chips {
display: flex;
flex-wrap: wrap;
gap: var(--space-1, 0.25rem);
align-items: center;
}
.tag-chip {
display: inline-flex;
align-items: center;
gap: 0.25em;
padding: 0.1em 0.5em;
font-size: var(--text-xs, 0.75rem);
line-height: 1.5;
background: var(--bg-secondary);
border: 1px solid var(--border);
border-radius: 999px;
color: var(--text-primary);
white-space: nowrap;
}
.tag-chip.readonly {
color: var(--text-secondary);
}
.tag-remove {
display: inline-flex;
align-items: center;
justify-content: center;
padding: 0;
width: 1.1em;
height: 1.1em;
border: none;
background: transparent;
color: var(--text-secondary);
font-size: 1.1em;
line-height: 1;
cursor: pointer;
border-radius: 50%;
}
.tag-remove:hover {
color: var(--text-primary);
background: var(--bg-hover, rgba(0, 0, 0, 0.06));
}
.tag-entry {
flex: 1;
min-width: 6ch;
border: none;
background: transparent;
color: var(--text-primary);
font-size: var(--text-sm, 0.875rem);
padding: 0.15em 0.1em;
outline: none;
}
.tag-empty {
font-size: var(--text-xs, 0.75rem);
color: var(--text-tertiary, var(--text-secondary));
}
.tag-suggestions {
position: absolute;
top: calc(100% + 2px);
left: 0;
z-index: 20;
min-width: 10rem;
max-width: 100%;
display: flex;
flex-direction: column;
background: var(--bg-primary);
border: 1px solid var(--border);
border-radius: var(--radius-md, 6px);
box-shadow: 0 4px 16px rgba(0, 0, 0, 0.12);
overflow: hidden;
}
.tag-suggestion {
text-align: left;
padding: 0.35em 0.6em;
border: none;
background: transparent;
color: var(--text-primary);
font-size: var(--text-sm, 0.875rem);
cursor: pointer;
}
.tag-suggestion:hover {
background: var(--bg-hover, var(--bg-secondary));
}
</style>
web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte
+202 -35
View File
@@ -17,6 +17,7 @@
import { userColor } from '$lib/collab/cursorColor';
import { authStore } from '$lib/stores/auth.svelte';
import FieldEditor from '$lib/components/fields/FieldEditor.svelte';
import TagInput from '$lib/components/fields/TagInput.svelte';
import ItemTimeline from '$lib/components/timeline/ItemTimeline.svelte';
import ChildItems from '$lib/components/ChildItems.svelte';
import BacklinksPanel from '$lib/components/BacklinksPanel.svelte';
@@ -97,6 +98,32 @@
let titleInputEl = $state<HTMLTextAreaElement>();
let fields = $derived<Record<string, any>>(item ? parseFields(item) : {});
// Tags live on item.tags (a JSON-array string), NOT in the schema. Parse
// defensively — the column defaults to "[]" but tolerate empty/garbage.
let tags = $derived.by<string[]>(() => {
if (!item?.tags) return [];
try {
const parsed = JSON.parse(item.tags);
if (!Array.isArray(parsed)) return [];
// Dedupe case-insensitively (keep first as typed). The write path
// doesn't enforce per-item uniqueness, so an API/imported item can
// carry ["ux","ux"]; cleaning here keeps rendering keys unique and
// the dedupe gets persisted on the next save.
const seen = new Set<string>();
const out: string[] = [];
for (const t of parsed) {
if (typeof t !== 'string') continue;
const key = t.toLowerCase();
if (seen.has(key)) continue;
seen.add(key);
out.push(t);
}
return out;
} catch {
return [];
}
});
let tagSuggestions = $state<string[]>([]);
let schema = $derived(collection ? parseSchema(collection) : { fields: [] });
let settings = $derived<CollectionSettings>(collection ? parseSettings(collection) : { layout: 'balanced', default_view: 'list' });
let layout = $derived(settings.layout);
@@ -284,11 +311,7 @@
// a debounced save is in flight, but a user
// mid-keystroke with no save yet pending would
// still lose chars without this branch.
if (collabProvider) {
item = updated;
} else {
item = { ...updated, content: item.content };
}
item = adoptServerItem(updated);
const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []);
if (!item || item.id !== reqItemId) return;
itemLinks = links;
@@ -301,11 +324,7 @@
try {
const updated = await api.items.get(reqWsSlug, reqItemSlug);
if (!item || item.id !== reqItemId) return;
if (collabProvider) {
item = updated;
} else {
item = { ...updated, content: item.content };
}
item = adoptServerItem(updated);
} catch {
// Ignore — will catch up on next event
}
@@ -347,11 +366,7 @@
// Same collab-aware adoption rule as the SSE
// handler above (TASK-1262).
if (!item || item.id !== reqItemId) return;
if (collabProvider) {
item = updated;
} else {
item = { ...updated, content: item.content };
}
item = adoptServerItem(updated);
const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []);
if (!item || item.id !== reqItemId) return;
itemLinks = links;
@@ -363,11 +378,7 @@
try {
const updated = await api.items.get(reqWsSlug, reqItemSlug);
if (!item || item.id !== reqItemId) return;
if (collabProvider) {
item = updated;
} else {
item = { ...updated, content: item.content };
}
item = adoptServerItem(updated);
const links = await api.links.list(reqWsSlug, updated.slug).catch(() => []);
if (!item || item.id !== reqItemId) return;
itemLinks = links;
@@ -447,7 +458,10 @@
api.collections.get(wsSlug, collSlug),
itemsPromise
]);
item = itemData;
// Overlay any in-flight optimistic tag edit so navigating away and
// back mid-save can't show (and then let a follow-up edit overwrite
// with) stale server tags. See withInflightTags. Per Codex PR #659.
item = withInflightTags(itemData);
collection = collData;
collectionStore.setActiveItem(itemData);
editorStore.resetForDoc();
@@ -800,7 +814,7 @@
// item; a navigation away should not stamp
// fresh content into the OTHER item's slot.
if (item && item.id === refreshCtx.itemId) {
item = fresh;
item = withInflightTags(fresh);
}
// Refetch succeeded → safe to rebuild.
forceRefreshNonce += 1;
@@ -1030,7 +1044,7 @@
if (!item || titleDraft.trim() === item.title) return;
saveStatus = 'saving';
try {
item = await api.items.update(wsSlug, item.id, { title: titleDraft.trim() });
item = withInflightTags(await api.items.update(wsSlug, item.id, { title: titleDraft.trim() }));
showSaved();
} catch {
saveStatus = 'idle';
@@ -1065,7 +1079,7 @@
try {
const fresh = await doUpdate(false);
if (item && item.id === targetItem.id) item = fresh;
if (item && item.id === targetItem.id) item = withInflightTags(fresh);
showSaved();
} catch (e) {
// BUG-1538 / TASK-1539: same open-children-guard recovery
@@ -1089,7 +1103,7 @@
return;
}
if (forced) {
if (item && item.id === targetItem.id) item = forced;
if (item && item.id === targetItem.id) item = withInflightTags(forced);
showSaved();
return;
}
@@ -1111,6 +1125,146 @@
}
}
// Serialized, coalescing tag saver. Tags are a top-level item column
// (item.tags), not a schema field, and the chip input makes rapid edits
// easy — so instead of firing concurrent PATCHes and guarding the races,
// we keep ONE save in flight per item and coalesce to the latest desired
// set. This structurally removes the overlap class: no stale completion
// can clobber a newer set, and `confirmed` always tracks the last
// server-acknowledged tags so a failed save reverts to server truth (not
// to an optimistic, unconfirmed value). Scoped by item id so a save still
// draining for a previous item can't touch the current one. Per Codex
// PR #659 rounds 1/4/5.
type TagSaver = {
itemId: string;
ws: string;
pending: string[] | null; // latest desired set not yet sent (coalesced)
desired: string[]; // latest desired set (in-flight OR pending) for reload reapply
running: boolean;
confirmed: string; // last server-acknowledged tags JSON (revert target)
};
// Keyed by item id so each item's in-flight saver stays discoverable across
// navigation — navigating away from A and back must find A's running saver
// and coalesce into it, not spawn a second concurrent A saver. Per Codex
// PR #659 round 6.
const tagSavers = new Map<string, TagSaver>();
function updateTags(newTags: string[]) {
if (!item) return;
const targetItem = item;
const targetWs = wsSlug;
// Optimistic so chips react instantly.
item = { ...item, tags: JSON.stringify(newTags) };
// Coalesce into this item's running saver if one exists; otherwise
// start a fresh one. The currently-displayed item is the only one whose
// tags can be edited, so targetItem.id keys the right saver.
const existing = tagSavers.get(targetItem.id);
if (existing && existing.running) {
existing.pending = newTags;
existing.desired = newTags;
return;
}
const saver: TagSaver = {
itemId: targetItem.id,
ws: targetWs,
pending: newTags,
desired: newTags,
running: false,
confirmed: targetItem.tags // confirmed baseline captured at burst start
};
tagSavers.set(targetItem.id, saver);
void flushTagSaver(saver);
}
async function flushTagSaver(saver: TagSaver) {
saver.running = true;
saveStatus = 'saving';
try {
while (saver.pending !== null) {
const toSave = saver.pending;
saver.pending = null;
const fresh = await api.items.update(saver.ws, saver.itemId, {
tags: JSON.stringify(toSave)
});
saver.confirmed = fresh.tags;
// Reconcile the UI to server truth only when nothing newer is
// queued (avoids flicker) and we're still on this item. Route
// through adoptServerItem so the tag PATCH echo can't clobber
// unsaved editor content — its response carries the server's
// `content`, and non-collab editors mirror item.content. Per
// Codex PR #659 round 11.
if (saver.pending === null && item && item.id === saver.itemId) {
item = adoptServerItem(fresh);
}
}
if (item && item.id === saver.itemId) showSaved();
// A newly-created tag should appear in autocomplete next time.
void loadTagSuggestions(saver.ws);
} catch (e) {
console.error('Failed to save tags:', e);
saver.pending = null;
if (item && item.id === saver.itemId) {
// Revert to the last server-confirmed tags, not the optimistic set.
item = { ...item, tags: saver.confirmed };
saveStatus = 'idle';
toastStore.show('Failed to save', 'error');
}
} finally {
saver.running = false;
// Drop the entry once fully drained so the map doesn't accumulate
// stale savers; guard on identity so a newer saver isn't evicted.
if (saver.pending === null && tagSavers.get(saver.itemId) === saver) {
tagSavers.delete(saver.itemId);
}
}
}
// Overlay an in-flight optimistic tag edit onto any server snapshot of an
// item before it's assigned to `item`. EVERY `item = <server data>`
// assignment (realtime SSE/sync, content-save echo, post-action refresh,
// initial load) must route through this: a tag PATCH owns `item.tags` until
// it drains, and the refresh handlers' `saveStatus === 'saving'` guard is
// racy (checked before their own await, not after), so overlaying the
// saver's latest desired set at assignment time is the only race-free
// guarantee that a concurrent snapshot can't drop the unsaved tags. Per
// Codex PR #659 rounds 8/9.
function withInflightTags(next: Item): Item {
const saver = tagSavers.get(next.id);
return saver?.running ? { ...next, tags: JSON.stringify(saver.desired) } : next;
}
// Realtime-refresh convenience: applies the content-adoption rule (under
// collab the editor reads Y.Doc so adopt server content verbatim; otherwise
// preserve the live local content) and the tag overlay.
function adoptServerItem(updated: Item): Item {
return withInflightTags(
collabProvider ? updated : { ...updated, content: item?.content ?? updated.content }
);
}
// Load the workspace's distinct tags for autocomplete. Pure data-fetch
// keyed on the workspace slug (see the $effect below) — kept separate from
// the item-load path per the Svelte 5 effect-splitting convention.
async function loadTagSuggestions(ws: string) {
if (!ws) return;
try {
const all = await api.tags.list(ws);
// Drop stale results: if the workspace changed while this request
// was in flight (page instance reused across navigation, or a
// post-save reload for a now-previous workspace), don't overwrite
// the current workspace's suggestions. Per Codex PR #659 round 2.
if (ws !== wsSlug) return;
tagSuggestions = all.map((t) => t.tag);
} catch {
if (ws === wsSlug) tagSuggestions = [];
}
}
$effect(() => {
loadTagSuggestions(wsSlug);
});
// stampSourceUrl writes the pad_source_url + pad_imported_at orphan
// keys into the item's `fields` JSON. The keys aren't declared in
// any collection schema — `internal/items/validate.go` only iterates
@@ -1157,7 +1311,7 @@
fields: JSON.stringify(merged)
});
if (item && item.id === targetItem.id) {
item = fresh;
item = withInflightTags(fresh);
}
} catch (err) {
// Non-fatal: the content was inserted regardless of whether
@@ -1270,7 +1424,7 @@
} else {
update.clear_assigned_user = true;
}
item = await api.items.update(wsSlug, item.id, update);
item = withInflightTags(await api.items.update(wsSlug, item.id, update));
showSaved();
} catch {
saveStatus = 'idle';
@@ -1288,7 +1442,7 @@
} else {
update.clear_agent_role = true;
}
item = await api.items.update(wsSlug, item.id, update);
item = withInflightTags(await api.items.update(wsSlug, item.id, update));
showSaved();
} catch {
saveStatus = 'idle';
@@ -1577,7 +1731,7 @@
// drop the queued edit. Mirrors the Round 8 fix in
// flushRawIfPending. Per Codex review round 9.
if (rawPendingMarkdown === toSave) {
item = updated;
item = withInflightTags(updated);
rawPendingMarkdown = null;
editorStore.setDirty(false);
showSaved();
@@ -1585,7 +1739,7 @@
// Newer pending edit; keep local content, adopt
// server-side metadata only. The next debounce
// cycle will land the queued edit.
item = { ...updated, content: item.content };
item = withInflightTags({ ...updated, content: item.content });
}
}).catch(() => {
if (!item || item.id !== reqItemId) return;
@@ -1666,13 +1820,13 @@
// from under the user's keystrokes and lose the
// queued edit. Per Codex review round 8.
if (rawPendingMarkdown === markdown) {
item = updated;
item = withInflightTags(updated);
rawPendingMarkdown = null;
} else {
// Newer edit pending — keep our local
// content but adopt server-side metadata
// (timestamps, version, modified_by).
item = { ...updated, content: item.content };
item = withInflightTags({ ...updated, content: item.content });
}
} catch {
saveStatus = 'idle';
@@ -1838,7 +1992,7 @@
}
function handleVersionRestore(updatedItem: Item) {
item = updatedItem;
item = withInflightTags(updatedItem);
}
async function handleDelete() {
@@ -1865,7 +2019,7 @@
itemLinks = itemLinks.filter(l => l.id !== linkId);
// Refresh item to update parent info
const refreshed = await api.items.get(wsSlug, itemSlug);
item = { ...refreshed, content: item.content };
item = withInflightTags({ ...refreshed, content: item.content });
toastStore.show('Relationship removed', 'success');
} catch (e: any) {
toastStore.show(e.message ?? 'Failed to remove relationship', 'error');
@@ -1913,7 +2067,7 @@
addLinkResults = [];
// Refresh item to update parent info
const refreshed = await api.items.get(wsSlug, itemSlug);
item = { ...refreshed, content: item.content };
item = withInflightTags({ ...refreshed, content: item.content });
toastStore.show('Relationship added', 'success');
} catch (e: any) {
toastStore.show(e.message ?? 'Failed to add relationship', 'error');
@@ -2343,6 +2497,19 @@
{/if}
{/each}
<!-- Tags (item.tags — spans collections, not a schema field) -->
<div class="field-row">
<span class="field-label">Tags</span>
<div class="field-value">
<TagInput
{tags}
suggestions={tagSuggestions}
onchange={updateTags}
readonly={!canEdit}
/>
</div>
</div>
<!-- Assignment: user + role -->
{#if workspaceMembers.length > 0 || agentRoles.length > 0}
<div class="fields-header" style="margin-top: var(--space-4)">Assignment</div>