GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Feature/offline sync conflict ux by priyanshuvishwakarma273403 · Pull Request #22 · docsgraph/docsgraph

Conversation

@priyanshuvishwakarma273403

@priyanshuvishwakarma273403 priyanshuvishwakarma273403 commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Overview

This PR designs and implements interactive connection status indicators, a dynamic offline editing flow, and a conflict resolution overlay modal that supports field-level Last-Write-Wins (LWW) and offline edit preservation without data loss.

Key Changes

1. Connection & Sync Status UI (apps/web/src/App.tsx)

  • Dynamic Indicators: Replaced static tags with stateful badges reflecting the client's network state:
    • Online & Synced (Blue): Client is connected with remote and local stores in parity.
    • Syncing Changes... (Amber): Spinners animates while updates are being pushed/pulled.
    • Offline Mode (Gray): Client operates locally against SQLite OPFS adapter.
    • ⚡ Sync Conflict! (Red Bounce): Warns users when locally modified fields diverge from remote server updates.
  • Simulated Connection Toggle: Added a toggle button in the header so developers and users can transition the workspace between online/offline connection states.

2. Interactive Offline Editing & Conflict Triggering

  • Inline Title Renaming: Added a "Rename" button next to document titles in the Reader header.
  • Offline State Accumulator: Updates made to titles while simulated offline are flagged (offlineEditMade). Reconnecting online simulates a sync cycle that prompts a sync conflict.
  • Manual Trigger: Added a "Force Conflict" button to instantly launch the conflict resolution modal.

3. Conflict Resolution Comparison Modal

  • Displays Local Offline edits side-by-side with Server updates.
  • Enables choosing either side:
    • Keep Local (Offline Edit): Preserves the locally stored value and schedules push.
    • Keep Remote (Accept Server): Discards the local value, overwriting the SQLite record with the server version.
  • Provides a "Resolve Later" button that hides the overlay but leaves the red ⚡ Sync Conflict! warning badge clickable in the top header.

Verification Results

  • pnpm lint: Passed (0 warnings/errors)
  • pnpm test: Passed (14/14 tests)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@priyanshuvishwakarma273403
priyanshuvishwakarma273403 requested review from Aidmann, BixbyCodes, DenizAltunkapan, GideonBa and MaximilianRau04 as code owners August 23, 2026 12:50
MaximilianRau04
MaximilianRau04 requested changes Aug 23, 2026
View reviewed changes

@MaximilianRau04 MaximilianRau04 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole flow never touches the real SyncManager from #18 — it's a self-contained mock wearing the real feature's clothes, and it has a state bug on top.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread apps/web/src/App.tsx Resolved

Uh oh!

There was an error while loading. Please reload this page.

priyanshuvishwakarma273403 added 3 commits August 24, 2026 13:56
@priyanshuvishwakarma273403
fix: resolve offline sync conflict UI issues, integrate SyncManager, …
6b345db
…and fix sqlite emulator update bug
@priyanshuvishwakarma273403
fix: resolve eslint no-explicit-any violations in App.tsx and store.ts
386a70a
@priyanshuvishwakarma273403
feat: implement real data-driven contract analysis and dynamic key de…
ae8e41b
…tails extraction
MaximilianRau04
MaximilianRau04 requested changes Aug 24, 2026
View reviewed changes

@MaximilianRau04 MaximilianRau04 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix on the main issue: this now genuinely integrates with SyncManager/store instead of fabricating conflicts, and the selectedDocId bug is gone since detection is store-driven now. One new bug introduced by this fix, otherwise ready.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx Outdated
await loadData();
}

store.clearConflicts();

@MaximilianRau04 MaximilianRau04 Aug 24, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clears the entire conflicts array unconditionally, but only resolves currentConflict (activeConflicts[0]). If getActiveConflicts() ever returns more than one (multiple diverged fields/entities), resolving the first one silently discards the rest without ever surfacing them to the user. Should remove just the resolved conflict from the array, not clear all of them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@priyanshuvishwakarma273403
Merge branch 'main' into feature/offline-sync-conflict-ux and resolve…
078879f
… conflicts
GideonBa
GideonBa requested changes Aug 30, 2026
View reviewed changes

@GideonBa GideonBa left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing the fake setTimeout conflict with a real one detected in applyRemoteOps is a solid step, and moving the Conflict type into the data package so the app stops redeclaring it inline is the right direction. The stray Tailwind classes that never existed (bg-slate-805, text-slate-405, hover:bg-slate-750) being cleaned up is a nice bonus.

A few things I would like addressed:

Blocking:

  1. Conflicts live only in memory on LocalStore, so reloading the page during a conflict loses it while the divergent local edit stays. For a feature about surviving offline, that is the case that matters most.
  2. getActiveConflicts hands back the internal array by reference and applyRemoteOps pushes into it, so React holds the same array it mutates and will not re-render for new conflicts.
  3. The else branch treats "field was edited locally" and "field is not in the column allowlist" as the same thing, so an unrecognized remote field pops a conflict dialog at the user.
  4. Overriding window.fetch from a component patches every request the app makes, not just sync.

Minor:

  • The row is re-queried inside the per-field loop.
  • baseUrl is hardcoded to localhost.
  • A shadowed snakeKey, and substr is deprecated.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/data/src/store.ts
}

export class LocalStore {
private activeConflicts: Conflict[] = [];

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else in LocalStore goes through the SQLite adapter, but conflicts are a plain field on the instance. So a refresh, a crash, or closing the tab while the conflict modal is open drops every pending conflict, and the local edits that caused them stay in the database still diverged from the server. The user is left with no indication anything needs resolving.

This wants a conflicts table alongside the others, written inside the same transaction as applyRemoteOps so a conflict cannot be detected and then lost. getActiveConflicts becomes async, which the two call sites in App.tsx are already positioned to handle since they sit in async functions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/data/src/store.ts

constructor(private adapter: SqliteAdapter) {}

getActiveConflicts(): Conflict[] {

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the live internal array. App.tsx passes the result straight into setActiveConflicts, so React state and this.activeConflicts are the same object, and line 606 then pushes into it. React compares by reference, sees no change, and does not re-render, so a conflict detected during an active sync silently never reaches the UI even though the array grew.

return [...this.activeConflicts] fixes the immediate problem. resolveConflict already reassigns rather than mutating, which is why resolution appears to work and detection does not.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/data/src/store.ts
@@ -563,6 +585,34 @@ export class LocalStore {
if (!locallyModifiedFields.has(key) && tableAllowed.has(snakeKey)) {

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition mixes two unrelated checks, and the else catches both. !locallyModifiedFields.has(key) failing means a genuine conflict. tableAllowed.has(snakeKey) failing means the remote sent a column this table does not have, which is a schema or version mismatch, not something the user can resolve by picking a side.

As written, an older client receiving an op with a new field shows a conflict dialog offering a choice between two values for a field it cannot store either way. Please split them:

if (!tableAllowed.has(snakeKey)) continue; // or log it
if (!locallyModifiedFields.has(key)) { /* apply */ }
else { /* conflict */ }

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx
// Intercept window.fetch to simulate offline behavior and direct sync calls to backend
useEffect(() => {
const originalFetch = window.fetch;
window.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaces the global fetch for the whole application, so every request, not only sync, throws when the simulated switch is off. Anything the app fetches for unrelated reasons fails in a way that looks like a network error, which will be confusing to debug.

It is also fragile as a component effect: the cleanup restores whatever originalFetch captured, so if anything else patches fetch in between, or this effect ever runs twice without its cleanup, the app is left with a stale or doubly wrapped implementation.

Since HttpSyncClient is constructed right here on line 35, it would be cleaner to give it an injectable fetch, or an offline flag the client checks, and leave the global alone. That keeps the simulation scoped to the thing being simulated.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/data/src/store.ts
updateKeys.push(`${snakeKey} = ?`);
updateValues.push(remotePayload[key] as SqlParam);
} else {
const entityRows = await tx.query(

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This SELECT * sits inside the loop over payload keys, so an op touching five conflicting fields queries the same row five times, and it re-reads state that the surrounding transaction may already have. Hoisting it above the loop, or reusing the row the caller already loaded to build locallyModifiedFields, gives you one read per entity.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread packages/data/src/store.ts
[op.entityId]
);
const currentEntity = entityRows[0];
const snakeKey = this.camelToSnake(key);

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this shadows the snakeKey declared on line 584 with an identical value. Harmless today, but it is the kind of thing that turns into a real bug the moment someone changes one of the two. Just use the outer one.

Also on line 607, substr is deprecated; slice(2, 11) does the same job. If conflict ids need to be stable across reloads once they are persisted, a deterministic id built from entity, field and remote value would serve better than a timestamp and random suffix, and would make the duplicate check on line 599 unnecessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx
let syncStatusListener: ((status: SyncStatus) => void) | null = null;

const syncClient = new HttpSyncClient({
baseUrl: 'http://localhost:8000',

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A hardcoded localhost URL means any build other than a developer's own points at nothing. import.meta.env.VITE_API_BASE_URL with this as the fallback keeps local development working and makes the deployed build configurable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx
const adapter = new InMemorySqliteAdapter();
const store = new LocalStore(adapter);

let syncStatusListener: ((status: SyncStatus) => void) | null = null;

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: a module-level mutable listener plus module-level syncClient and syncManager means only one component can ever subscribe, and the listener is null whenever no component is mounted, so status changes during that window are dropped. If SyncManager exposed subscribe and unsubscribe returning an unsubscribe function, the effect could use it directly and this variable would go away.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread apps/web/src/App.tsx
// Trigger Mock Sync Conflict button
const handleTriggerMockConflict = () => {
// Trigger Mock Sync Conflict by applying a simulated remote divergent operation
const handleTriggerMockConflict = async () => {

@GideonBa GideonBa Aug 30, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, and I realise a demo trigger is useful while the backend conflict path is still settling. Two things worth tidying if it stays: the store.updateDocument(doc.id, { title: doc.title }) on the next lines is a write of the value onto itself purely to mark the field dirty, which will confuse anyone reading it without a comment saying so. And sequence: 999 on line 184 is a magic number that will collide with real sequences once the server is producing them.

Ideally this button and its handler live behind a development-only flag so they do not ship in a production build.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MaximilianRau04 MaximilianRau04 MaximilianRau04 requested changes
@GideonBa GideonBa GideonBa requested changes
@DenizAltunkapan DenizAltunkapan Awaiting requested review from DenizAltunkapan DenizAltunkapan is a code owner
@BixbyCodes BixbyCodes Awaiting requested review from BixbyCodes BixbyCodes is a code owner
@Aidmann Aidmann Awaiting requested review from Aidmann Aidmann is a code owner

Requested changes must be addressed to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@priyanshuvishwakarma273403 @MaximilianRau04 @GideonBa
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.