Skip to content

Architecture restructure, background sync, and 1.0.21 - #16

Merged
Punlork merged 60 commits into
mainfrom
refactor/architecture-restructure
Sep 28, 2026
Merged

Punlork merged 60 commits into
mainfrom
refactor/architecture-restructure

Conversation

@Punlork

@Punlork Punlork commented Sep 28, 2026

Copy link
Copy Markdown
Owner

Moves the app to a pub workspace (app shell + six packages) with one offline sync engine behind every feature, fixes eleven bugs (four lost or corrupted data), and turns background sync on for 1.0.21.

main This branch
Packages 0 6
Tests 6 files, suite did not compile 32 files, 165 passing
Analyzer 24 issues, 5 errors 3 issues, 0 errors
CI release only analyze + every suite; release waits for it

Highlights

  • Offline sync: saves never wait for the server; lists never fetch on a tap; pulls delete safely; a sync pill shows what is waiting or failed.
  • Data-loss fixes: a refresh no longer undoes unsynced edits (a247225); loans keep their customerId (79a15ee); prices no longer vanish (094aa51).
  • Build-time feature flags; income off in production.
  • Shop form and listing rebuilt for phone width and Khmer.
  • Melos, CI check job, generated copyWith, pruned dependencies.

Merging releases 1.0.21: the version is bumped, and the push to main runs the release job.
Rollback for background sync: FEATURE_BACKGROUND_SYNC: false in config/features/production.json, then release again.

Full recap, including what is not yet verified and what is still open: docs/BRANCH_RECAP.md

🤖 Generated with Claude Code

Punlork and others added 30 commits September 18, 2026 16:50
Phase 0 of docs/ARCHITECTURE_RESTRUCTURE.md. Mechanical only, no logic
changes, so the diff stays reviewable.

- pubspec name my_app -> jabhouy; imports rewritten across 90 files
- build_runner regenerated 268 outputs
- test/income_service_test.dart: IncomeService gained an ApiService as
  its first positional parameter, so every argument was shifted by one.
  Adds _MockApiService and passes it first.
- dart fix applied 22 lint fixes in 14 files, mostly directives_ordering
  churn caused by the rename sorting earlier than my_app
- .github/copilot-instructions.md barrel-import examples updated

flutter analyze: 24 issues with 5 errors -> 3 issues with 0 errors.
flutter test: did not compile -> 11 pass, 1 fail.

The remaining failure is real and pre-existing, not caused by this
commit. income_service.dart:445 writes syncStatus synced when
triggerRemoteSync is false, so notifications imported from the native
backlog are marked uploaded without ever being sent, and the backlog
query at :366 only looks at pending and error. It lands on its own
commit because this one must stay mechanical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_upsertNotificationModel wrote syncStatus synced whenever
triggerRemoteSync was false. That flag means "do not upload this now",
so the row was recorded as already on the server having never been
sent. importPendingTrackedNotifications is the caller that hits it,
which is every notification captured natively while the app was
backgrounded.

Nothing recovered them. The backlog query in _loadPendingNotifications
selects only pending and error, so a row marked synced is invisible to
every later sync attempt. The income data was silently dropped.

Both branches want pending: when the caller does push, it overwrites
with synced or error immediately afterwards; when it does not, the row
is genuinely waiting. The ternary had no reason to exist, which also
leaves triggerRemoteSync unused inside the method, so it is removed
rather than left as a parameter that no longer affects anything.

Proven by the existing test 'saveTrackedNotificationMap keeps imported
notifications pending', which could not run until the suite compiled
again in the previous commit.

flutter test: 12 pass, 0 fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First package of the restructure. The repository becomes a pub
workspace; jabhouy_core holds the schema and depends on drift only, so
it builds and tests without Flutter.

The split follows what actually needs Flutter. The tables, the
@DriftDatabase and the migration strategy have no platform dependency
and move to the package. Opening the file does -- dart:io,
path_provider, sqlite3_flutter_libs -- so _openConnection stays in the
app as openAppDatabaseConnection in
lib/app/service/database/database_connection.dart.

AppDatabase now has one constructor that takes its executor. The
no-arg constructor hid the file-open inside the schema, and
AppDatabase.forTesting existed only to work around it; both are gone.
Dependency injection passes the real executor, tests pass
NativeDatabase.memory().

Also here:
- analysis_options no longer excludes packages/**, which would have
  left every extracted package silently unanalysed -- the opposite of
  why they are being extracted
- 10 importers repointed at package:jabhouy_core/jabhouy_core.dart
- dart fix cleaned the directives_ordering churn that caused

Known cost, and the reason the doc flagged phase 5: build_runner does
not cross workspace members. Regenerating drift now needs a run in the
package as well as at the root.

dart analyze in jabhouy_core: no issues.
flutter analyze: 3 issues, 0 errors, all pre-existing in lib/home.
flutter test: 12 pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The package boundary was resting on a lint. Every workspace member
shares one package_config, so a stray `package:flutter` import inside
jabhouy_core resolves fine and the analyzer stays quiet;
depend_on_referenced_packages reports it as an info nobody has to act
on.

`dart test` is the enforcement. The plain Dart VM has no dart:ui, so a
Flutter import stops the suite loading at all. Verified by adding
package:flutter/widgets.dart to app_database.dart and watching the
suite fail to load, then reverting.

The four tests are chosen to be worth keeping on their own:
- the schema opens at version 5 and every declared table exists
- a shop item round-trips with the category it references
- a duplicate BankNotifications fingerprint is rejected, which is the
  invariant the outbox engine will generalise to every entity
- clearUserData empties every table

dart test in jabhouy_core: 4 pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Goal one of the restructure doc: no syncStatus integer literals in
feature code. 51 of them across 11 files are gone; a grep for
syncStatus paired with 0, 1 or 2 now returns nothing outside generated
code.

SyncStatus lives in jabhouy_core with a Drift TypeConverter, so the
column stays INTEGER and the wire values stay 0, 1 and 2. No migration,
no schema version bump.

fromWireValue resolves an unrecognised integer to pending, not synced.
A row we cannot classify has no evidence it ever reached the server,
and the commit before last was a bug where exactly that assumption
dropped income data.

The type change is what found the rest of the work. Attaching the
converter turned 71 analyzer errors on, each one a place an untyped int
was crossing a boundary: four models declaring `int syncStatus`, the
NotificationSyncStatusUpdater typedef, a loaner form defaulting to 0,
and every companion literal. Drift comparisons moved from equals(1) to
equalsValue(SyncStatus.pending).

income_service's private _notificationSyncStatusSynced, Pending and
Error constants are deleted rather than retyped; they existed only to
name the integers the enum now names.

flutter analyze: 3 issues, 0 errors, all pre-existing in lib/home.
flutter test: 12 pass. dart test in jabhouy_core: 4 pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The queue the four syncPendingChanges() clones never had. Schema goes
to 6; the migration creates the table and its index, so no existing
data is touched.

syncStatus describes a row. An outbox entry describes a job, which is
what lets it carry the three things a status column has nowhere to put:

- attemptCount and nextAttemptAt, so a failed push is retryable instead
  of terminal
- lastError, replacing the catch (_) blocks that collapsed every
  distinct failure into the integer 2
- dependsOnLocalId, so a shop item waits for the offline category it
  references rather than racing it

entityType and operation are textEnum columns, not strings, so an
invalid job cannot be written. fingerprint generalises the one
idempotency guarantee the app already had, on BankNotifications.

entityLocalId is text rather than integer. Entity tables are moving to
UUID primary keys to kill the -(millis % 1000000) collision; until then
it holds the stringified integer id, and the column does not change
when they do.

A unique key on (entityType, entityLocalId) means a second write to the
same row updates the pending job instead of queueing a duplicate push.

The index is declared with @TableIndex rather than a standalone Index
object. A standalone Index only runs in the migration path, so a fresh
install would silently have no index -- caught by the test that reads
sqlite_master.

dart test in jabhouy_core: 9 pass. flutter analyze: 3 issues, 0 errors.
flutter test: 12 pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third package, pure Dart, 8 tests under `dart test`. Generalises
firebase_income_sync_service.dart -- the one sync path in the app that
does not lose writes -- and adds what it lacked.

Carried over from income: concurrent pushes of one fingerprint coalesce
onto a single future, recently-succeeded fingerprints sit in a bounded
LRU, every transition writes a diagnostic entry.

New:
- attemptCount and nextAttemptAt give exponential backoff with a
  ceiling, so a failed push is retryable rather than terminal
- dependsOnLocalId is honoured, so a shop item waits for the offline
  category it references
- SyncPushOutcome splits transient failure from rejection. The old code
  had one case: every failure became syncStatus = 2 and stopped. A 500
  and a 400 need opposite handling and now get it
- a delete result is checked like any other push, replacing the
  hardcoded ApiResponse(success: true)

On the port boundary: SyncTransport and SyncDiagnostics are abstract
because a network client is the one part that genuinely varies and the
one part that needs Flutter in production. The outbox itself is read
through AppDatabase directly -- it is pure Dart and in-memory testable,
so wrapping it in a port would add indirection without buying a seam.

The tests found three bugs in the engine before this landed:
- insertOnConflictUpdate targets the primary key, so a second edit to a
  row hit the unique constraint instead of folding into the pending job.
  Now an explicit DoUpdate on (entityType, entityLocalId), which also
  resets the attempt count: fresh bytes, fresh schedule.
- drain() ran a single pass, so clearing a category left its items for
  the next call. It now repeats until a pass clears nothing.
- drift reads DateTime columns back as local time, which a UTC literal
  in the backoff test did not match.

dart analyze: no issues in either package. flutter analyze: 3 issues, 0
errors, all pre-existing in lib/home. Tests: 12 app, 9 core, 8 sync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc described a plan; three of its decisions changed on contact with
the code. Recording them here rather than leaving the doc describing a
shape the repository no longer has.

- tables live in jabhouy_core, because drift's generated companions
  co-locate with @DriftDatabase and a feature-owned table would be a
  package cycle -- and because ShopItems.categoryId proves the schema is
  shared anyway
- packaging came before layering for core, which was a move not a
  refactor
- workspace members share one package_config, so the boundary is
  enforced by dart test rather than by resolution
- build_runner does not cross workspace members

The diagram claimed OutboxEntries lives in the engine. It lives in core;
the engine holds the drain loop, backoff and the transport port.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 2 of docs/ARCHITECTURE_RESTRUCTURE.md. ShopService was 380 lines
owning Drift queries, HTTP calls, sync orchestration and row mapping in
one class, with no layer between the bloc and the database. It splits
into the three things it was doing:

- ShopDao   -- every Drift statement, and the only place that knows
               ShopItems is a table rather than a list of models
- ShopApi   -- every HTTP call, and nothing else
- ShopRepository / DefaultShopRepository -- local-first orchestration,
               behind an interface so the bloc can be tested on a double

Shop earns no logic/ layer: none of the three conditions in the doc are
met today, so the repository is the seam.

ShopDao is a plain class, not a @DriftAccessor. The schema lives in
jabhouy_core and the app generates nothing today; a generated mixin here
would reference tables across a workspace member, which is the phase 5
cross-package codegen risk. It would buy the select()/update() shorthand
and nothing else, so phase 5 can decide it on purpose.

Two of the seven data-loss defects close as a consequence:

- a rejected delete no longer reports success. The old drain loop
  hard-coded ApiResponse(success: true) in the delete branch, so an item
  the server refused to delete was marked synced and reappeared on the
  next pull. It now stays queued as failed. A row the server never saw
  is purged locally without a DELETE that would only 404.
- reconciling a created row runs in one transaction. The local delete
  and the server insert were two statements, so a crash between them
  lost the row from the phone and from the queue.

The localOnly flag is gone from the public surface. It existed so the
drain loop could re-enter the write methods; deciding "am I the user's
write or the engine's push?" is the repository's business, not the
caller's.

8 tests against a real in-memory database, one per behaviour that can
lose a write. Full suite green, analyze reports no new issues.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure move, no logic change: the target layout in
docs/ARCHITECTURE_RESTRUCTURE.md puts screens, widgets and bloc in ui/,
and shop is the reference slice the rest of the features copy. Landing
it separately from the previous commit keeps that diff readable.

models/ stays at the feature root: it is read by ui/ and data/ alike,
and CategoryItemModel belongs to category, which is still a nested
feature awaiting its own slice. service/category_service.dart stays for
the same reason.

Everything outside the feature imports through shop.dart, so the only
non-barrel edit is one deep import between two shop widgets.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The table claimed 3b was done. The engine is built and its 8 scenario
tests pass, but nothing in lib/ calls it and all four
syncPendingChanges() clones are still live, which is what phase 3's
"done when" actually asks for. Recording that rather than letting a
green row hide it.

Also records the two decisions phase 2 made -- a plain-class DAO instead
of a @DriftAccessor, and which two of the seven defects closed as a
consequence of rewriting shop's drain loop rather than moving it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc uses "outbox" more than twenty times and never says what it is,
and four nouns for "a write that has not reached the server" are live at
once -- syncStatus the int, SyncStatus the enum, OutboxEntry and
SyncPushOutcome. Each was correct when added and none replaced the last,
because phase 3 is unfinished, so a reader meets four words for one idea
with no signal about which is current.

States which noun is the queue, which are scaffolding, and that the
OutboxEntries/OutboxEntry pair is Drift's convention rather than a
distinction the design is drawing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three renames, all inside jabhouy_core and jabhouy_sync, which have no
callers in lib/ yet -- so the blast radius is four files and their tests.

- OutboxEntries.fingerprint -> idempotencyKey. "Fingerprint" says the
  value identifies something; it does not say why you would want that.
  The name came from BankNotifications, where the value literally is a
  hash of notification text and the name fits. On a generic outbox the
  column means "push twice under this key and the server admits it
  once", so it says that. BankNotifications.fingerprint is unchanged.
- SyncPushFailedTransiently -> SyncPushRetryable. Twenty-six characters
  to say "try again", next to a sibling named SyncPushRejected that
  already implies "do not".
- OutboxEntries.entityLocalId -> localId. The entity is already named by
  the column beside it.

Not renamed: entityType and SyncEntityType. The earlier suggestion was
tableName, which would have been wrong -- the enum values are shopItem
and bankNotification, model names rather than table names, and the doc
picked them so a reader meets no new vocabulary.

Schema 6 -> 7 recreates the outbox table rather than renaming columns in
place. That is safe precisely here: schema 6 has shipped on no device and
no code has ever enqueued a job, so the queue is guaranteed empty. A
device below 6 creates the table and then recreates it, which is
harmless.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The engine had no callers. Shop is the first, and the only feature with
a seam to put it behind. Its own drain loop is gone:
DefaultShopRepository now writes the row and enqueues an OutboxEntry in
the same call, and syncPendingChanges() is engine.drain().

Three pieces make the seam:

- FeatureSyncAdapter -- one per feature, knows that a shopItem create is
  POST /items. Phase 4 adds siblings rather than growing a switch.
- AppSyncTransport -- the app's SyncTransport implementation, routes a
  job to the adapter for its entity type. Everything HTTP lives on this
  side, which is what keeps jabhouy_sync pure Dart.
- ShopSyncAdapter -- sends the job and applies the answer to the row.

ApiResponse gained a statusCode. ApiException has always carried one and
ApiService has always discarded it in its catch blocks, which left every
caller unable to tell a 500 from a 400. The engine is the first caller
that must: 5xx, 408, 429 and a missing status are retryable, everything
else is rejected and stops. A row now stays pending while a retry is
still coming and only becomes failed once the server has said no on the
merits -- the old code wrote failed on the first hiccup and never looked
again.

A 404 on delete counts as done. It is the state the job was trying to
reach.

dependsOnLocalId is set for items filed under an offline-created
category. Category has no adapter yet so nothing is queued under that
id and it is inert today; it becomes load-bearing when category is
layered, with no change here.

4 new tests for what the old loop could not express: a 500 reschedules
with a backoff instead of failing, a 400 stops and keeps its reason, two
edits before the first push are one job, and a 404 on delete is done.
41 tests green across the three packages.

Still live: the syncPendingChanges() clones in category, customer and
loaner. Phase 3 closes when those three have adapters too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 3 turns out not to be a phase. A feature can only be wired to the
engine once it has a repository to wire, so phase 3 and phase 4 are one
piece of work repeated per feature rather than two stages. Recording
that instead of leaving a row that cannot be completed in order.

Also records the FeatureSyncAdapter seam, the ApiResponse.statusCode
addition and which four of the seven defects are now closed for shop and
still open everywhere else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second clone deleted. CategoryService splits the same way ShopService
did -- CategoryDao, CategoryApi, CategoryRepository and a
CategorySyncAdapter -- and CategoryBloc holds the repository.

This is the adapter that makes dependsOnLocalId load-bearing. Shop has
been setting it since 071cbe1, but with no category jobs in the queue
nothing was ever waited on. Now an item filed under an offline category
cannot reach the server first, and the test enqueues the two jobs in the
wrong order on purpose -- natural insertion order would pass without any
dependency logic at all.

CategoryDao.reconcileCreated does the half ShopDao.reconcileCreated
cannot: a category is the only table anything else references, so when
its negative id is swapped for the server's, every shop item pointing at
the old id is repointed in the same transaction. A crash partway through
would otherwise leave items referencing a category that no longer
exists.

Category stays inside lib/shop/ rather than becoming lib/category/. Its
UI is shop's UI -- the chips, the dropdown and the filter sheet are shop
widgets -- and phase 5 extracts jabhouy_shop, which would take category
with it either way. The doc calls category a feature to say how it is
layered, not where it lives.

5 new tests, 46 green across the three packages.

Still live: the clones in customer and loaner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fifth of the seven defects closed, and records why category came before
customer and loaner: dependsOnLocalId was inert until a second adapter
existed, so the FK-ordering fix is the one that needed exactly this
feature next rather than any of the three.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure move, matching shop in 0768f99. Everything outside the feature
imports through customer.dart, so the barrel is the only edit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third clone deleted; only loaner's is left. Same split as shop and
category -- CustomerDao, CustomerApi, CustomerRepository and a
CustomerSyncAdapter -- with CustomerBloc on the repository.

CustomerDao.reconcileCreated repoints loaners the same way
CategoryDao.reconcileCreated repoints shop items: Loaners.customerId
references Customers.id, so swapping a negative local id for the
server's has to fix a foreign key, in the same transaction as the swap.

That repoint is tested now even though loaner has no adapter yet,
because it is a property of the DAO rather than of the queue. What is
not yet testable is the ordering half -- a loaner created offline
against an offline customer -- which needs loaner's adapter, exactly as
the shop/category pair needed category's.

5 new tests, 51 green across the three packages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One clone left. Loaner is also the feature that earns a logic/ layer, so
it closes phase 3 and opens phase 4 in the same slice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure move, matching shop in 0768f99 and customer in b196cf1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fourth clone is gone. Phase 3's "done when" -- four
syncPendingChanges() deleted -- is met: the three remaining matches in
lib/ are repository methods that delegate to SyncEngine.drain(), not
forty lines of loop each.

Loaner is the first feature to earn a logic/ layer, on the first of the
doc's three conditions: it merges two repositories. A loan response
carries its customer embedded, and caching that customer is worth doing
-- the autocomplete reads it -- but LoanerRepository writing the
Customers table is the reach that made the old services untestable in
isolation. RefreshLoanersUseCase pulls the page through
LoanerRepository and hands the embedded customers to
CustomerRepository.cacheCustomers. That is the whole logic/ folder: one
use case, not one per operation.

Fixes a live crash. Loaners.customer is a denormalised JSON blob and the
encoder wrote the raw syncStatus into it. That was an int until c391ebb
made it a SyncStatus enum, after which jsonEncode threw
JsonUnsupportedObjectError on every loan carrying a customer -- so
saving one failed outright, and inside syncPendingChanges the catch (_)
swallowed it into syncStatus = 2. encodeCustomer now writes
syncStatus.wireValue, with a regression test named for the round trip.

LoanerDao.reconcileCreated has no foreign key to repoint: nothing
references Loaners.id. Loaner is the leaf of the three-table chain that
category and customer sit above.

9 new tests, 60 green across the three packages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four clones deleted. Records what actually ordered the work: the
dependency graph, not size. dependsOnLocalId is inert with one adapter
registered, so each feature's adapter was only worth as much as the
adapter of whatever it references.

Also records the loaner JSON crash alongside the phase 0 income fix --
both found by rewriting a path rather than reading it -- and that
logic/ is not yet Flutter-free, which is the argument for landing
Result<T> in jabhouy_core next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last outstanding row of the doc's core primitives table, and the
thing that makes "no Flutter in logic/" enforceable rather than
aspirational: a use case cannot avoid app.dart while ApiResponse is the
only way to say "this might have failed".

Result<T> is sealed with Ok(value) and Err(AppException). ApiResponse
made a caller check success and then write data!, two steps the compiler
could not connect, so the ! was load-bearing and unchecked. Here the
value only exists inside Ok.

AppException replaces ApiException, which lived in the app package and
so could not be named from jabhouy_sync. It carries the status code and
answers isRetryable -- no status means the request never landed, 5xx is
the server's problem, 408 and 429 ask for a repeat, everything else will
say the same thing next time. That judgement is currently duplicated in
outcomeFor(); the next commit deletes the copy.

Result.guard wraps a throwing call so a caller never has to handle two
shapes of failure.

19 core tests, all under dart test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every fromJson in the app leaned on tryCast and let, and both lived in
lib/app/models/pagination_model.dart -- so no model could be read
without importing the app package, which carries Flutter. That is the
reason logic/ could not be Flutter-free: the chain ran use case ->
barrel -> model -> app.dart -> Flutter.

The file had no imports of its own, so the lift is exact. Six models and
DAOs now import jabhouy_core instead of app.dart, and the analyzer
confirms none of them needed anything else from it.

43 app tests and 19 core tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the core primitives table. ApiResponse{success, data} made a
caller check success and then write data! -- two steps the compiler
could not connect, so the ! was load-bearing and unchecked. The shop,
category, customer and loaner repositories now return Result<T>, and
there is no ! left on any of those paths.

The repositories also stopped returning a message. "Saved offline. It
will sync when you are back online." was chosen inside the data layer
from a connectivity check taken BEFORE the push was attempted. Now
_settle() re-reads the row after draining and reports what actually
happened -- a row missing from under its local id was reconciled onto
the server's, so it is synced; a row still there says pending or failed
-- and syncFeedback() in the ui layer turns that into words. The
message is no longer a guess.

AppException.isRetryable replaces the copy of that judgement in
outcomeFor(), so jabhouy_sync and the app agree by construction.

logic/ is Flutter-free, and test/architecture/logic_layer_test.dart
walks the transitive import closure to keep it that way. It failed on
its first run: loaner_model.dart imported the customer barrel, which
exports customer's ui folder, which reaches flutter/material -- three
hops from a file that looks like a plain data class. That is the
category of leak the rule was written for and the reason review cannot
catch it.

ApiResponse stays for auth, profile, upload, fcm and income, which are
not layered yet. They convert at the edge of an api/ class, so nothing
above data/ knows the older shape exists.

71 tests green: 44 app, 19 core, 8 sync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The core primitives table is complete. Also records why four things that
had no Flutter dependency of their own were nonetheless stuck in the app
package, and that the offline message was a guess taken before the push
rather than a report of it.

Adds the rule the architecture test taught us on its first run: inside a
feature, import the file, not the barrel. A barrel exports the feature's
ui folder, so it is a Flutter leak for anything that must stay pure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure move, matching the other three features. The last one, so every
feature folder now has the same shape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 4. IncomeService goes from 560 lines to 291: IncomeDao took the
Drift statements, IncomeApi the pull and its four-shaped response
parser, and PullRemoteNotificationsUseCase the pull policy. What is
left is what only it can do -- the Android bridge, the Firebase
session, device-role gating and the demo seed.

Closes the doc's open question. OutboxEntries does not supersede
BankNotifications.syncStatus; income registers an adapter and keeps the
column as a display hint, exactly as the other four features now do.
The fingerprint is both localId and idempotencyKey, because income is
the feature the outbox was generalised from: the UNIQUE constraint on
BankNotifications.fingerprint is the guarantee the other four had to
have one invented for.

What the engine adds to the one sync path that already worked:
FirebaseIncomeSyncService replayed the entire backlog on every
connectivity change, with no attempt count and no delay, so a server
that was down got hammered once per network blip. Uploads are jobs now,
with backoff.

A device that is not the main device answers retryable rather than
rejected, so the job waits and goes out if the device is promoted
instead of being dropped. A Firebase upload that does not confirm is
also retryable: that path answers with a bool, so nothing distinguishes
"network down" from "document refused", and dropping a recorded sale is
the outcome worth avoiding.

IncomeDiagnostics is a port, because NotificationDiagnosticsService
reaches the Android bridge through the income barrel and logic/ may not
carry Flutter. The service satisfies it as it stands.

The two existing income tests now run through the real dao, engine and
adapter rather than a mocked database, because they are the record of
what income must keep doing. 8 new tests beside them. 79 green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Income registers an adapter and keeps BankNotifications.syncStatus, so
syncStatus is a display hint everywhere and OutboxEntry is the queue
everywhere. Records why, and the two judgements in IncomeSyncAdapter
worth arguing with: a non-main device and an unconfirmed Firebase
upload are both retryable rather than rejected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Punlork and others added 29 commits September 21, 2026 15:32
go_router 18.0.1, four majors in one step. No source changes: nothing
in lib/ used an API these versions removed. The 14 call sites are
context.go, context.goNamed, context.push and context.pushNamed, all
unchanged across the range.

The doc calls this the largest behavioural risk in the plan, and
analyze cannot speak to it -- the suite has 52 tests and none touch the
router. So it was run on the iPhone 17 Pro Max simulator, which
exercises the three things the doc flagged:

- redirect reads AuthBloc through BlocProvider.of(context). AuthBloc is
  provided in app.dart above MaterialApp.router, so the redirect's
  context can still reach it; AuthBloc settled on Unauthenticated and
  the app redirected /home -> /signin, which is that path working.
- GlobalContext.currentContext = context inside pageBuilder ran, on the
  signin route.
- CustomTransitionPage rendered.
- DebuggerRouteObserver is still accepted in observers.

No exceptions in the run log.

Not exercised, and not claimable: pushNamed with extra, the nested
routes under /home, and the authenticated branch of redirect. All three
need a login this checkout has no credentials for, and there is no tap
automation on this machine. The router has no test coverage either way,
which is the gap phase 7 should close rather than a new one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both upgrades were free -- four go_router majors and a bloc major with
no source changes. The risk note was right about where to look and
wrong about the cost, so it is corrected rather than deleted.

Adds the honest entry to the testing table: routing has no coverage.
Phase 6 was verified by running the app on a simulator, which does not
survive into CI, and app_routes.dart holds a redirect that decides
every navigation in the app.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 5 cannot start while shop imports the app package, because the app
barrel exports app_routes.dart and the router imports every feature. Any
widget reached through that barrel drags the whole feature graph behind
it. Three separate things were holding the edge open.

Two of shop's cross-feature imports were not shop's to begin with.
settings_page.dart is 482 lines reading blocs from five features --
shop, category, customer, income and auth -- and lived under
lib/shop/ui/views/ only because the settings button sits in the shop
header. It moves to lib/settings/. shop_header.dart carried two auth
BlocListeners: signout succeeded -> tell AuthBloc -> go to signin. The
signout is dispatched from the settings page, two features away, and was
reacted to inside a shop widget. They move to home_page.dart, which
hosts ShopHeader and stays mounted for the whole signed-in session, so
the listener's lifetime widens rather than narrows.

The third is the app barrel itself, and that is what jabhouy_ui is for:
theme, assets, nine shared widgets, two mixins, the snack bars and the
loading overlay. Two of its files were trapped rather than shared.
GlobalContext lived in app_routes.dart because the router's pageBuilder
assigns it, which put a snack bar's dependency inside the file that
wires every feature; holding the value is not the router's job, only
assigning it is. TabScrollManager was declared inside home_page.dart and
read by shop, loaner and income, so three features imported the home
barrel for one InheritedWidget. All three of those imports are now gone.

Four widgets read AppLocalizations, which a package cannot reach. It
came to four strings -- loading, noItemFound, cancel, imgFound -- so the
package declares a UiStrings port and the app satisfies it from its ARB
files, the same shape as IncomeDiagnostics and SyncTransport. Choosing
the words stays where the translations are.

test/architecture/ui_package_test.dart enforces "no app imports",
because the pubspec does not: workspace members share one
package_config, so package:jabhouy/... resolves inside the package and
the analyzer stays quiet. Same softness logic_layer_test.dart exists
for. Verified by adding a banned import and watching it fail.

Shop is down from ~120 app-symbol references to ~60 across 13 files.
What remains is not UI: UploadBloc, BaseService/ApiService,
ConnectivityService, syncFeedback/outcomeFor and the AppRoutes name
constants. Those are the next commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shop's remaining pull on the app package was not UI. This commit moves
what was left, which turned out to belong in four different places.

Three were free moves into packages that already existed. logger.dart
depends on nothing but the logger package, and syncFeedback already
imported only jabhouy_core -- both go there. feature_sync_adapter.dart,
holding FeatureSyncAdapter and outcomeFor, imported only jabhouy_core
and jabhouy_sync, so it goes into jabhouy_sync where the engine that
calls it lives. None of the three had a reason to be in the app beyond
where they were first written.

jabhouy_net is new: ApiService with its four parts, BaseService,
UploadService, ConnectivityService and NetworkInspectorService. It
depends on jabhouy_ui, which is the wrong direction and is recorded
rather than fixed -- ApiService shows a snack bar and a loading overlay
on failure, so transport is choosing words. The alternative is a second
port for one call site. There is no cycle: jabhouy_ui imports nothing
from jabhouy_net, and the test below now proves it.

That constraint decided UploadBloc. It is shared presentation -- shop and
profile both drive it -- so it belongs in jabhouy_ui, and jabhouy_ui
cannot reach UploadService without creating the cycle Dart forbids. So
the bloc takes an ImageUploader port and the app joins the two ends with
UploadImageAdapter. Joining packages that must not name each other is
what an app shell is for.

The architecture test is now package_independence_test.dart and runs over
all four packages instead of one. Verified the way the last one was, by
adding a banned import to jabhouy_net and watching it fail.

Shop now reaches outside itself for two things: AppRoutes, in three
files, and AppLocalizations, in twelve. The second decides whether
jabhouy_shop is extracted at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan drew three packages and phase 5 produced six. None of the extra
three was a preference; each was the only way out of a cycle the app
package had been hiding.

jabhouy_l10n moves the ARB catalog and context.l10n out of lib/. A
package cannot reach lib/l10n/, and putting words on screen is most of
what a feature package does -- shop needed AppLocalizations in twelve
places. That deleted the UiStrings port added one commit earlier: an
interface, an InheritedWidget scope and an app-side adapter for four
strings, all of it there because the catalog could not move. It could. A
port is what you build when a dependency cannot move, and that is worth
checking before building one.

Route names leave app_routes.dart for jabhouy_core. They sat beside the
GoRouter config, so reading one name cost the whole feature graph. The
names are shared vocabulary and the wiring is the app shell's, and only
the first half has to be reachable from a package; what stays behind is
AppRouter. AdapterSyncTransport (was AppSyncTransport) moves to
jabhouy_sync for the same reason, next to the FeatureSyncAdapter it
routes to -- it imported nothing else.

jabhouy_shop is then a move. Its two repository test files move with it,
so the package tests itself: 17 tests under packages/jabhouy_shop/test.
package_independence_test.dart covers all six packages and was verified
the usual way, by planting a banned import and watching it fail.

The phase 5 risk was named correctly and priced wrong. Cross-package
Drift codegen cost nothing: tables live in jabhouy_core, shop's DAO is a
plain class, and build_runner in core after the split rewrote no source
at all. The lost time went to the app barrel, which the risk list never
mentioned.

Analyzer at its 3-issue baseline with zero errors. 41 app tests, 17 shop,
19 core, 8 sync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 4's income row cited 09b8a2a, the pre-amend copy of 6503310 that no
branch contains. Phase 5's row stopped at a3c7367 and missed 1550cfe,
where the phase actually closed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Income tracking is not ready to ship, and hiding its tab would have left
the rest of it running: the listener service, the device-role card, the
diagnostics route and the engine adapter all reach it from outside
lib/income.

Every read goes through FeatureFlags in jabhouy_core, never through
bool.fromEnvironment directly, so moving a flag to Remote Config is a new
implementation registered in DI rather than a hunt through call sites.
BuildTimeFeatureFlags reads --dart-define-from-file=config/features/
<flavor>.json, and a missing define reads as off: a build that forgot its
file hides unfinished work instead of shipping it.

Gradle decodes the same dart-defines into a manifest placeholder, so the
production APK ships BankNotificationListenerService with
android:enabled="false" -- checked in the merged manifests of real
production and development builds. One file drives both Dart and native.

Home picked tabs by index in four places. It now switches on HomeTab, and
scroll controllers stay one per slot, because shop, loaner and income ask
TabScrollManager for slots 0, 1 and 2 by number.

The income sync adapter stays registered with the flag off. Without an
adapter the engine rejects a job for good, so a job queued before the
flag flipped would be lost; with the flag off nothing new is queued.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Phase 0 renamed the Dart package and missed FLAVOR_APP_NAME, which all
nine build configurations still set to the Flutter template's "My App".
Android names itself in build.gradle, which is why only iOS showed it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
PUT /loans failed with a 400: the server validates createdAt as
YYYY-MM-DD and toJson sent a local timestamp. The time was never kept
anyway -- loans come back at 07:00 local, which is midnight UTC here.

toIsoDate() moves into jabhouy_core and replaces the private
_rfc3339Date the list filters already used. It formats the local day on
purpose: converting to UTC first would file a loan made before 07:00
under the day before.

This predates the restructure (c3cc38c). It surfaces now because the
engine keeps a rejected job and its reason, where the old sync loop wrote
syncStatus = 2 and moved on. The queued job re-reads the row and rebuilds
the body on every attempt, so it goes through on the next drain.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The loans API sends "customer": {"id": 24, ...} and no flat customerId.
fromJson only looked for a flat id, so every pull stored customer_id as
null -- and so did every successful push, because the adapter writes the
server's response back over the row. A loan kept its customer's name and
lost its id, and the next edit was a 400.

fromJson now reads the nested id. _toModel falls back to the stored
customer blob, which repairs rows already written that way without a
fresh pull; found on a real device where two of three loans had it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Six packages and an app had no single command to test them, because two
of them must run under dart test -- jabhouy_core and jabhouy_sync forbid
Flutter by design -- and the rest under flutter test. CI ran neither, so
package_independence_test and logic_layer_test only held while someone
remembered to run them.

`dart run melos run test` runs both runners across the workspace, and a
planted failure exits 1. `gen` runs build_runner wherever it is a
dependency. run:dev/stg/prod and build:prod bake in each flavor's feature
flag file.

Melos 7 lives in the root pubspec. Filters sit on `melos exec` rather
than on the scripts: a script-level packageFilters prompts for a package
and crashes without a terminal, and `steps` shells out to a global
`melos` that `dart run` does not provide.

sdkPath points at FVM's pinned SDK; CI has no .fvm/ and sets
MELOS_SDK_PATH=auto. The new check job reads the Flutter version from
.fvmrc so the analyzer baseline matches local, and the release job now
needs it to pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FcmService, IncomeService and NotificationDiagnosticsService each guarded
initialize() with a bool set before their first await. A second caller
saw the bool and returned at once, while setup was still running --
free to read diagnostics entries that had not loaded, or tracking status
before the sync service existed.

AsyncMemoizer hands every caller the same future. The new test holds the
bridge's load open and checks the second caller is still waiting; it
fails against the old bool.

IncomeService.dispose() swaps in a fresh memoizer where it used to reset
the bool, so a new session initializes again. `async` exports its own
Result, which collides with jabhouy_core's, so it is imported with
`show AsyncMemoizer`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CategoryDropdown found its selected value with firstWhere and no orElse,
so a filter whose category was no longer in the list -- deleted, or
missing while the list reloads -- threw "Bad state: No element" during
build. firstWhereOrNull shows no selection instead; the new widget test
throws exactly that StateError against the old line.

The package also replaces the income list's hand-rolled putIfAbsent
grouping with groupBy, which keeps the same order. The pie chart's sort
stays as it is: it already sorts a fresh list in place, and collection
would not make it shorter.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Thirteen hand-written copyWith methods become @copyWith(). Four of them
-- CustomerLoaded, IncomeLoaded, LoanerLoaded, ShopLoaded -- carried
their own `Object? x = _xUnset` sentinels and `as` casts so a filter or
banner could be cleared; the generator does the same thing with
$CopyWithPlaceholder, typed, and those sentinels are gone.

The risk ran the other way. A plain `x ?? this.x` keeps the field when
handed null; the generated method sets it, because nullability comes
from the constructor parameter. LoanerModel's `DateTime? createdAt` would
even reach the constructor's `?? DateTime.now()`. To find every call
that could notice, each such parameter was made temporarily non-nullable
and the analyzer listed the calls passing a possibly-null value: one,
loaner_view.dart:164, where both readings give the same result.

The analyzer then called six `syncMessage: null` arguments redundant.
It reads the generated interface, whose default looks like null; the
implementation's default is the placeholder, so removing them would
leave "Back online. Syncing..." on screen for good. They stay, each
with an ignore that says why, and loaner_state_copy_with_test pins both
halves: null clears, omission keeps.

Generated files are committed, as app_database.g.dart already is,
because the CI check job tests without running gen. They are excluded
from analysis: their positional-bool lints are the generator's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… restructure

ARCHITECTURE.md describes the system as built -- the package graph and
what may import what, the layers inside a feature, one edit traced from
an offline form to PUT /loans, the three push outcomes, feature flags,
generated code, and which test holds each rule. Its known-gaps list is
checked against the code: four repositories still mint colliding local
ids, a rejected job is re-sent on every drain because its schedule never
advances, and BackoffPolicy's comment says 5s where the engine waits 10s.

CLAUDE.md replaces .github/copilot-instructions.md as the one source for
assistants, which now points at it. The old file described a codebase
that no longer exists: no repository layer, syncPendingChanges() clones,
lib/shop/, lib/l10n/arb. It also carries the traps this branch found:
generated copyWith's null semantics, nested customer JSON, date-only
createdAt.

The README drops a coverage badge claiming 100%. It came from the
template's first commit and nothing has ever measured coverage.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Nine dependencies go. animated_text_kit, cloud_firestore and dio were
imported nowhere in the workspace -- no Kotlin uses Firestore either,
and the native firebase-firestore line in build.gradle pins its own BoM.
connectivity_plus, dropdown_button2, flutter_localizations,
flutter_staggered_grid_view, super_clipboard and transparent_image are
imported only by packages that already declare them; their plugins still
register, checked in GeneratedPluginRegistrant after a debug build.

flutter_launcher_icons moves to dev_dependencies: it is a CLI, never
imported. `generate: true` goes with the root l10n it served, which
moved to jabhouy_l10n. sqlite3_flutter_libs stays with a comment: it has
no Dart import because it only ships the native SQLite Drift opens.

Dependencies are grouped by category, alphabetical within each group,
so sort_pub_dependencies is off in the root analysis_options.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
OFFLINE_SYNC.md: saves return after the local write, lists stop calling
the server on search, filter and scroll, and the engine pushes and pulls
in the background one at a time. It names a data-loss bug that predates
the restructure: a pull writes the server's copy over a row whose edit
is still queued, and the queued job then sends that older copy.

SHOP_ITEM_FORM.md: the variant flow stays; labels move above fields,
cards collapse on create without escaping validation, edit keeps one
card with a Single/Pack toggle, and prices stop vanishing when they are
not ASCII digits.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The seller marks a loan paid while the push is failing -- offline,
retrying, or rejected as loan 38 was before 79a15ee. Any search, filter
or scroll then refreshes the list, and the four cacheServer* methods
wrote the server's copy over every row with insertOrReplace, unpaid and
marked synced. The queued job re-reads the row before each attempt, so
its next attempt sent that older copy, the server accepted it, and the
edit was gone with nothing reported. A queued delete came back the same
way, with isDeleted: false.

This predates the restructure: shop_service.dart at 0a9508e wrote pulls
identically.

A row belongs to the phone while the outbox holds a job for it, and a
job leaves only once the server confirms it, so a push in flight counts.
AppDatabase.queuedLocalIds reads that set, and each cacheServer* skips
those rows inside one transaction with the read, so an edit cannot land
between check and write.

Five tests, each failing against the old DAO: an unsent loan edit and an
unsent loan delete survive a refresh, and so do a customer rename, a
shop price change and a category rename; a loan with no queued job
still takes the server's copy.

Step 1 of docs/OFFLINE_SYNC.md.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The variant card squeezed three prices into one row, so the Khmer labels
for customer and seller price both truncated to the same "តម្លៃសម្រាប់…",
errors truncated with them, and a fixed field stayed red until the next
save. Labels now sit above their fields; customer price takes a row and
cost and seller share the next; errors wrap to two lines.

Prices went through int.tryParse, so 1,500, 1500.5 or Khmer digits
១៥០០ saved as no price with no error. WholeNumberFormatter maps Khmer
digits to ASCII and keeps digits only, and a price the form cannot read
is refused.

On create, adding a card collapses the ones that could already save to
one line (`ទំនិញរាយ · 400 រៀល`), and a card with a problem shows ⚠. The
fields of a collapsed card are Offstage, not removed, because
Form.validate() only checks mounted fields; save opens every broken card
and scrolls to the first. ShopItemVariantDraft.problems drives that from
the text alone, and expansion stays out of snapshot(), so opening a card
is not an unsaved change.

A Single/Pack toggle replaces typing a pack size into a single item, so
edit keeps that ability with one card that never collapses. The Custom
chip goes: it added a single card, and the variant name covers it. Pack
size must be at least 2.

Validation runs per field on user interaction, not on the Form: at Form
level every field validated once any one changed, and a render of the
Khmer form showed an untouched price red after typing a pack size.
CustomTextFormField gains an optional autovalidateMode for that; null
keeps its eight other callers as they were.

English "Default Price (per unit)" becomes "Cost Price", matching Khmer
តម្លៃដើម and per-variant pricing. 13 tests drive the real page.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Step 2 of docs/OFFLINE_SYNC.md: the engine side of saves that stop
waiting, used by nothing new yet.

drain() now runs one at a time. A call while one runs joins it and makes
it loop once more, so a job queued mid-drain is pushed by that drain
rather than left for the next trigger. _running clears in the same step
as the last _again check, so no call can land between them and be lost.
requestSync() is drain() without the wait, for saves.

SyncEngine.activity reports whether a drain is running and how many jobs
are waiting and failing, starting from the current state; the sync
indicator reads it. The engine does not know about connectivity, and the
indicator combines the two.

A rejected job was re-sent on every drain, since its schedule never
moved: the same bytes, refused again, once per save or refresh. It is
now parked until releaseRejected(), which bootstrap() calls at launch,
so a build that fixes how a body is made still gets to send it -- the
path loan 37 recovered by. An edit to the row re-enqueues it at once, as
before.

BackoffPolicy's comment said the first retry waits base; the engine asks
for attemptCount + 1, so it waits twice base. The comment now says so,
and both gaps leave ARCHITECTURE.md.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Step 3a of docs/OFFLINE_SYNC.md, behind Feature.backgroundSync: on in
development and staging, off in production.

Saving a loan, customer, shop item or category held a full-screen
overlay while _settle awaited SyncEngine.drain(), which pushes every due
job for every feature, each allowed 30 seconds. connectivity_plus calls
wifi without internet online, so one save could freeze the screen for
30 seconds per queued job. With the flag on, _settle calls requestSync()
and returns the local row; the list redraws from Drift when the push
lands, and the ten overlays in the loan, category and shop-item handlers
do not show. Offline, nothing is pushed, as before.

A save that returns early cannot say whether it reached the server, so
syncFeedback takes inBackground and returns the plain "Created"/"Updated"
message instead of "Saved offline"; the sync indicator in step 3d is
what reports the server's side.

Repositories and blocs take FeatureFlags as an optional argument that
defaults to all off, so every existing test and the flag-off path run
unchanged. DI and the router pass the real flags.

Two tests: a save returns while a push the transport holds open is still
pending, and the row reconciles to the server id once it lands; offline,
no push is tried.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Step 3b of docs/OFFLINE_SYNC.md. Nothing calls pull() yet; SyncCoordinator
in 3c does, behind Feature.backgroundSync.

SyncEngine.pull() drains first, then walks the pull adapters parents
first -- category, customer, shop item, loan -- pulling each list that is
older than 15 minutes, or every one with force. SyncCursors (schema 8)
records when each last pulled; a failed pull records nothing, so the next
trigger retries. clearUserData clears it, or the next account to sign in
would skip its first pull.

Drains and pulls now queue behind one lock. Without it, a create that
landed mid-pull would be missing from the list the pull downloaded, and
the pull's delete step would remove it.

A pull deletes a local row only when three things hold: the list is
provably complete, the row has a positive id, and it has no queued job.
"Complete" asks for two agreeing signals in fetchEveryPage -- no next
page and a row count matching the server's total -- because
Pagination.fromJson defaults a missing totalPages to 1, which alone makes
page 1 look like everything. Tracing that found two fallbacks that would
have passed a malformed reply off as an empty, complete list:
fetchItems' carried total: 0, and fetchCategories returned [] for a
non-list. Both now read as incomplete or as an error. The old per-load
refresh keeps cacheServer*, which never deletes.

List fetches take quiet, and the pull passes it: fetches show an error
snackbar by default, and nobody is waiting on a background pull.

Tests: fetchEveryPage's completeness rule six ways; the engine's order,
window, force, failure and a save waiting out a pull; a loan, item and
category the server deleted leaving the phone while queued and
never-sent rows stay; an uneven count deleting nothing; and a 7 -> 8
migration on a real file, which fails with "no such table" without the
migration step.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tself

Step 3c of docs/OFFLINE_SYNC.md, behind Feature.backgroundSync.

Searching, filtering, scrolling and opening a tab re-point the Drift
watch and send nothing; the old path fetched a page on every one. The
loan, shop, customer and category blocs skip their fetch branches, and
their reconnect handlers only clear the offline state. Pull-to-refresh
is the one load that asks the server, through each repository's
pullLatest(): a forced pull of that list, and shop's pulls categories
with it because the shop tab shows both.

SyncCoordinator starts syncing after sign-in, on AppLifecycleState
.resumed and on reconnect, and stops on sign-out. Each start is a pull,
which drains first; the engine's 15-minute window keeps it to one
download per list however often these fire.

SyncEngine.retryNow() runs on reconnect. connectivity_plus calls wifi
without internet online, so pushes made there fail and back off for up
to 30 minutes; a real reconnect is new evidence, and waiting out that
schedule would leave the seller's changes unsent. Rejections stay
parked: a reconnect does not change what the server refused.

Also removes an unused import df2e54e committed in migration_test.dart.

Tests: the loan bloc's search, filter and scroll touch neither the
refresh use case nor the server and pull-to-refresh pulls; the
coordinator pulls once however many times it is started, retries then
pulls on reconnect, stays quiet offline and after sign-out; retryNow
releases a backed-off job and not a rejected one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Step 3d of docs/OFFLINE_SYNC.md, and the last of step 3.

With background sync on, a pill rides on home's bottom bar, whose
background is transparent, so it sits just above it on any safe area.
It reads SyncEngine.activity and connectivity, and shows the most
pressing of: "N changes failed", which stays and opens a sheet of each
row with the server's own reason; "N changes waiting" while offline;
"Syncing…" only for a push past one second, so a quick save does not
flash it; and "Synced" for 1.5 seconds after a visible sync. Background
pulls say nothing. Its text is in jabhouy_l10n, English and Khmer.

It lives in lib/app/widget rather than jabhouy_ui, as the plan had it:
it reads SyncEngine, and jabhouy_ui has no other reason to depend on
jabhouy_sync.

Loan, customer and shop item rows gain SyncStateIcon: nothing when
synced, a cloud when pending, an error mark when failed. It shows
whatever the flag, since syncStatus is true in both modes; until now no
card showed it, so a pending loan looked synced.

With the flag on, the loan, customer and shop lists stop setting their
offline banner; the pill says how many changes wait instead.

Tests: a sub-second push never shows "Syncing…"; a slow one shows it,
then "Synced", then nothing; offline counts what waits; online with a
queue and no failure stays hidden; failures stay up and a tap lists the
reason. Rendered in Khmer to check the pill reads in full.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…swipe

Swiping pull-to-refresh repeatedly on the shop tab queued a full
download per swipe. Three things combined:

- Shop's refresh events were restartable. Each swipe cancelled the
  bloc's handler, but not the pull that handler had already handed to
  the engine.
- SyncEngine.pull() did not coalesce: every call waited its turn behind
  the lock and downloaded every page again.
- onRefresh sent an event and returned at once, so the spinner vanished
  and nothing stopped the next swipe.

The engine now keeps at most one pull running and one waiting; a request
that arrives while one waits folds into it. Requests are kept as a set
of forced lists and a set of lists to check for staleness, so folding a
forced loan pull into a stale check of everything does not force every
list. A forced pull of a list pulled under 10 seconds ago is skipped
(chosen, not measured). Ten swipes during a pull now download the list
once; before, ten times.

With background sync on, the loan and shop lists' refresh() returns the
pull itself, so the spinner stays until the download ends and
RefreshIndicator ignores swipes meanwhile. Flag off, refresh() sends the
same event as before.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The grid card put the whole stored name on one line with no ellipsis,
so the part that tells variants apart -- the label after " - " -- was
what got cut: two variants of AwknLKSNDjk1 read as the same "AwknLKSNDjk1
-" card. A full-size logo filled about 60% of every card for the 6 of 8
items with no photo, four cards to a screen, and prices ran ungrouped.
The list card showed "$400.00 / unit": dollars, for a riel price per
variant.

Both cards now show the product name (grid: two lines, list: one, then
an ellipsis), the variant label with its pack size on the next line --
the label shrinks and "×12" always shows, and an item with no label says
Single or Pack -- one price, and the category chip when there is one.
Items without a photo get a neutral tile with their first letter at the
same height as a photo, so aligned rows still line up; about seven cards
fit where four did. A tint per name came out light grey or near-black in
this theme and was dropped.

formatRiel groups digits and uses ៛, and a missing price is "—", never
0; the detail sheet uses it too. A price too long to fit scales down.
productName and variantLabel move onto ShopItemModel, and the form
reads them, so one rule splits the stored name.

Tests pin the format, the split on the real stored names, and each
overflow rule at 170 and 360 wide with the worst of the data. Rendered
in Khmer at iPhone width with the real items to check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
df2e54e took the schema to 8 with SyncCursors.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Step 5 of docs/OFFLINE_SYNC.md. The seller ran the development build on
a phone first, which was step 4. Production now saves without waiting
for the server, reads lists from the phone, pulls in the background, and
shows the sync pill.

Rolling back is FEATURE_BACKGROUND_SYNC: false in production.json and a
new release; schema 8 needs no downgrade. The flag-off path is now dead
in every flavor, and step 6 deletes it one release after this one.

Adds docs/BRANCH_RECAP.md, a summary of the branch for the pull request.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Punlork
Punlork merged commit 743a7eb into main Sep 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant