Skip to content

feat(mobile): add native tag management and collapsible tabs - #188

Closed
koreyba wants to merge 12 commits into
mainfrom
feature-mobile-tag-management
Closed

koreyba wants to merge 12 commits into
mainfrom
feature-mobile-tag-management

Conversation

@koreyba

@koreyba koreyba commented Aug 1, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add a native mobile Tags tab with web-parity search, alphabetical filtering, rename, delete, and empty-state behavior
  • support complete online tag data plus typed offline queue/replay for tag mutations
  • add a collapsible bottom tab bar that is visible on open, hides while scrolling down, and reappears while scrolling up
  • prevent superseded failed bulk queue operations from being replayed after compaction
  • document requirements, design, implementation, testing, deployment, and monitoring decisions

Validation

  • mobile regression: 706/706 tests passed across 82 suites
  • root unit/integration/web coverage: 1,418/1,418 tests passed across 183 suites
  • focused Allure checks: 22/22 core and 18/18 mobile tests passed with no findings
  • root and mobile type-check passed
  • root and mobile ESLint passed
  • npx ai-devkit@latest lint --feature mobile-tag-management passed
  • git diff --check passed before commit

Android/iOS device smoke testing remains unavailable in this environment.

Codacy static-analysis quality-gate findings remain intentionally out of scope for this PR fix.

Summary by CodeRabbit

  • New Features

    • Added mobile tag management with searchable, alphabetized tag lists, usage counts, filtering, renaming, deletion, and note navigation.
    • Added offline-capable tag updates with optimistic changes and automatic synchronization.
    • Added a collapsible mobile tab bar that responds to scrolling.
  • Bug Fixes

    • Improved offline queue cleanup, retry handling, and synchronization reliability.
    • Preserved pending local notes during remote data replacement.
  • Documentation

    • Added comprehensive mobile tag management planning and release documentation.
  • Tests

    • Expanded coverage for tag management, offline updates, synchronization, navigation, data persistence, and tab-bar behavior.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: babcdc00-dbc5-4729-9bef-77eb6222934e

📝 Walkthrough

Walkthrough

Adds native mobile tag management with complete-note tag summaries, search, filtering, rename, delete, optimistic updates, offline queueing, and synchronization. Adds typed bulk tag services and persistence handling. Adds a scroll-driven collapsible tab bar across mobile routes.

Changes

Mobile tag management

Layer / File(s) Summary
Tag services, queue contracts, and persistence
core/services/notes.ts, core/types/offline.ts, core/utils/compactQueue.ts, core/services/offlineSyncManager.ts, ui/mobile/services/database.ts
Adds paginated note retrieval, bulk tag operations, typed queue payloads, queue compaction, discarded-entry cleanup, and local note replacement.
Tag derivation and mutation synchronization
ui/mobile/utils/tagManagement.ts, ui/mobile/hooks/useTagManagement*.ts, ui/mobile/services/sync.ts
Adds normalized tag summaries, online and offline loading, optimistic mutations, rollback, queueing, cache invalidation, and replay dispatch.
Tags screen and native components
ui/mobile/app/(tabs)/tags.tsx, ui/mobile/components/tags/*, ui/mobile/types/lucide-react-native.d.ts
Adds the Tags route, search and letter filtering, tag cards, rename and delete modals, themed styles, and icon declarations.
Feature specifications and release support
docs/ai/{requirements,design,implementation,planning,testing,deployment,monitoring}/*
Adds feature requirements, design, implementation, testing, deployment, monitoring, and planning documentation.

Collapsible mobile tab bar

Layer / File(s) Summary
Tab-bar state and provider
ui/mobile/utils/collapsibleTabBar.ts, ui/mobile/providers/CollapsibleTabBarProvider.tsx
Adds threshold-based visibility state, reset behavior, provider context, and consumer hook.
Screen wiring and animated layout
ui/mobile/app/(tabs)/_layout.tsx, ui/mobile/app/(tabs)/*, ui/mobile/components/search/*, ui/mobile/components/settings/*
Connects list scroll events to the provider, resets state on route changes, animates the tab bar, and registers the Tags tab.
Validation and build support
ui/mobile/tests/*, ui/mobile/tsconfig.json, ui/mobile/eslint.config.mjs, AGENTS.md
Adds component, integration, unit, service, and persistence coverage, plus mobile declaration/build configuration and worktree restrictions.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TagsScreen
  participant useTagManagementMutations
  participant NoteService
  participant MobileSyncService
  TagsScreen->>useTagManagementMutations: submit rename or delete
  useTagManagementMutations->>NoteService: persist bulk tag operation online
  useTagManagementMutations->>MobileSyncService: queue operation offline or after retryable failure
  MobileSyncService->>NoteService: replay queued operation
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary changes: native mobile tag management and collapsible tabs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature-mobile-tag-management

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Status

Updated for PR #188 at a60de0b on 2026-08-20T20:04:57.968Z.

📊 Allure Test Report

Open Allure Report

Contributing Workflows

Workflow Status Suites
Unit Tests ✅ Completed Core Unit, Core Integration, Web Unit, Mobile Unit
Component Tests ✅ Completed Web Component
E2E Tests (PR Preview) ✅ Completed Web E2E

Catalog: All reports

🤖 Android Build Panel

Check a box below to trigger a release build:

  • 🚀 Build Stage Release APK
  • 🚀 Build Prod Release APK

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Deploying everfreenote with  Cloudflare Pages  Cloudflare Pages

Latest commit: a60de0b
Status: ✅  Deploy successful!
Preview URL: https://a65d2e98.everfreenote.pages.dev
Branch Preview URL: https://feature-mobile-tag-managemen.everfreenote.pages.dev

View logs

@github-actions

github-actions Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

Allure Report Summary

Name Duration Stats New Flaky Retry Report
Allure Report Allure Report 5m 25s Passed tests 2762 1 0 0 View

@github-actions

github-actions Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

Qodana for JS

It seems all right 👌

No new problems were found according to the checks applied

@@ Code coverage @@
+ 91% total lines covered
11792 lines analyzed, 10799 lines covered
+ 94% fresh lines covered
365 lines analyzed, 346 lines covered
# Calculated according to the filters of your coverage tool

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

@koreyba

koreyba commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review please

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@koreyba

koreyba commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review please

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@koreyba
koreyba marked this pull request as ready for review August 2, 2026 08:02

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
ui/mobile/app/(tabs)/tags.tsx (1)

75-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate duplicate rename validation.

performRename (lines 75-81) and handleRenameSubmit (lines 95-100) repeat the same !renameTarget and empty-replacement checks. handleRenameSubmit re-validates before calling performRename, which validates again. If validation rules change later, a future edit can update one function and miss the other, causing inconsistent behavior between the merge-confirmation path and the direct-save path.

Extract the shared validation into one helper and call it from both call sites.

♻️ Proposed refactor to share validation
+  const validateRenameInput = useCallback((): string | null => {
+    if (!renameTarget) return null
+    const replacement = renameValue.trim()
+    if (!replacement) {
+      setRenameError('Tag name cannot be empty')
+      return null
+    }
+    return replacement
+  }, [renameTarget, renameValue])
+
   const performRename = useCallback(() => {
-    if (!renameTarget) return
-    const replacement = renameValue.trim()
-    if (!replacement) {
-      setRenameError('Tag name cannot be empty')
-      return
-    }
+    if (!renameTarget) return
+    const replacement = validateRenameInput()
+    if (!replacement) return

     renameTag.mutate(
       { tag: renameTarget.name, replacement },
       {
         onSuccess: () => setRenameTarget(null),
         onError: (mutationError) => {
           Alert.alert('Rename failed', mutationError.message)
         },
       }
     )
-  }, [renameTarget, renameValue, renameTag])
+  }, [renameTarget, renameTag, validateRenameInput])

   const handleRenameSubmit = useCallback(() => {
-    if (!renameTarget) return
-    const replacement = renameValue.trim()
-    if (!replacement) {
-      setRenameError('Tag name cannot be empty')
-      return
-    }
+    if (!renameTarget) return
+    const replacement = validateRenameInput()
+    if (!replacement) return

     const mergesExistingTag = allTags.some((tag) => (
       tag.name.trim().toLocaleLowerCase() === replacement.toLocaleLowerCase()
       && tag.name.trim().toLocaleLowerCase() !== renameTarget.name.trim().toLocaleLowerCase()
     ))
     ...
-  }, [allTags, performRename, renameTarget, renameValue])
+  }, [allTags, performRename, renameTarget, validateRenameInput])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/mobile/app/`(tabs)/tags.tsx around lines 75 - 120, Extract the shared
rename-target and trimmed replacement validation from performRename and
handleRenameSubmit into a single helper, preserving the existing empty-name
error behavior and return value needed to stop submission. Call that helper from
both functions, while keeping merge detection and confirmation in
handleRenameSubmit and mutation execution in performRename.
ui/mobile/components/tags/AlphabeticalIndex.tsx (1)

7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Unused-parameter lint noise on callback type signatures. Codacy repeatedly flags underscore-prefixed parameter names inside function type declarations (not runtime parameters) as unused. The shared root cause is one ESLint configuration gap: the no-unused-vars rule does not recognize named parameters in type-only positions, so it flags names that exist purely for documentation.

  • ui/mobile/components/tags/AlphabeticalIndex.tsx#L7: onSelect: (_letter: string | null) => void — remove the parameter name or configure the rule to ignore type-signature parameters.
  • ui/mobile/components/tags/TagManagementCard.tsx#L8-L9: onPress: (_tag: string) => void and onActions: (_tag: MobileTagSummary) => void — same fix.
  • ui/mobile/components/tags/TagSearchInput.tsx#L8-L8: onChangeText: (_value: string) => void — same fix.
  • ui/mobile/app/(tabs)/tags.tsx#L292-L292: onChangeValue: (_value: string) => void — same fix.

Fix the ESLint rule configuration once (e.g., disable no-unused-vars for type-only parameter positions, or switch to unnamed parameter types like (value: string) => void without the underscore convention, since it isn't needed here) rather than patching each file individually.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/mobile/components/tags/AlphabeticalIndex.tsx` at line 7, Update the shared
ESLint no-unused-vars configuration to ignore named parameters used only in
TypeScript function type signatures, preserving normal unused-variable checks.
Leave the callback type declarations unchanged in
ui/mobile/components/tags/AlphabeticalIndex.tsx:7 (onSelect),
ui/mobile/components/tags/TagManagementCard.tsx:8-9 (onPress and onActions),
ui/mobile/components/tags/TagSearchInput.tsx:8 (onChangeText), and
ui/mobile/app/(tabs)/tags.tsx:292 (onChangeValue); these sites require no direct
changes.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/services/notes.ts`:
- Around line 171-195: Extract the duplicated changed-note persistence logic
from renameTag and deleteTag into a shared helper that invokes updateNote
concurrently for all changed notes, using Promise.all or an appropriate
bounded-concurrency approach. Update both methods to reuse this helper while
preserving their existing tag transformation and return behavior.

In `@docs/ai/implementation/feature-mobile-tag-management.md`:
- Line 9: Update the placeholder status statement in the
feature-mobile-tag-management implementation guide to reflect the current
implemented structure and validation evidence already documented in the
surrounding sections. Remove the stale Phase 1/requirements-review wording while
preserving the existing implementation details.

In `@docs/ai/planning/feature-mobile-tag-management.md`:
- Line 89: Confirm the single verified full mobile regression total from the
final CI or Allure result, then use that same number in all affected documents:
replace the total in docs/ai/planning/feature-mobile-tag-management.md at lines
89-89, docs/ai/implementation/feature-mobile-tag-management.md at lines 27-27,
and docs/ai/testing/feature-mobile-tag-management.md at lines 60-60.

In `@ui/mobile/hooks/useTagManagement.ts`:
- Around line 45-52: Update the successful remote-load path in the note-fetching
flow to replace or reconcile the user-scoped local snapshot on every getAllNotes
result, including an empty array; do not guard databaseService.saveNotes(notes)
behind notes.length. Add a regression test covering an existing non-empty local
cache followed by an empty remote result and verify subsequent local fallback
returns no stale notes.

In `@ui/mobile/hooks/useTagManagementMutations.ts`:
- Around line 101-118: Update the catch handling in the tag mutation flow around
NoteService.renameTag/deleteTag to log the caught error and only call
queueBulkMutation() for genuinely retryable connectivity failures. Propagate
permanent server-side failures to the caller instead of treating them as
successfully queued, while preserving offline queuing for transient network
errors.

---

Nitpick comments:
In `@ui/mobile/app/`(tabs)/tags.tsx:
- Around line 75-120: Extract the shared rename-target and trimmed replacement
validation from performRename and handleRenameSubmit into a single helper,
preserving the existing empty-name error behavior and return value needed to
stop submission. Call that helper from both functions, while keeping merge
detection and confirmation in handleRenameSubmit and mutation execution in
performRename.

In `@ui/mobile/components/tags/AlphabeticalIndex.tsx`:
- Line 7: Update the shared ESLint no-unused-vars configuration to ignore named
parameters used only in TypeScript function type signatures, preserving normal
unused-variable checks. Leave the callback type declarations unchanged in
ui/mobile/components/tags/AlphabeticalIndex.tsx:7 (onSelect),
ui/mobile/components/tags/TagManagementCard.tsx:8-9 (onPress and onActions),
ui/mobile/components/tags/TagSearchInput.tsx:8 (onChangeText), and
ui/mobile/app/(tabs)/tags.tsx:292 (onChangeValue); these sites require no direct
changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f1f29b43-08e5-4fa6-854b-8eecf2f9b1c5

📥 Commits

Reviewing files that changed from the base of the PR and between a3752e3 and 498621b.

📒 Files selected for processing (50)
  • core/services/notes.ts
  • core/tests/services/tag-management-notes.test.ts
  • core/tests/unit/tag-mutation-queue.test.ts
  • core/types/offline.ts
  • core/utils/compactQueue.ts
  • docs/ai/deployment/feature-mobile-tag-management.md
  • docs/ai/design/feature-mobile-tag-management.md
  • docs/ai/implementation/feature-mobile-tag-management.md
  • docs/ai/monitoring/feature-mobile-tag-management.md
  • docs/ai/planning/feature-mobile-tag-management.md
  • docs/ai/requirements/feature-mobile-tag-management.md
  • docs/ai/testing/feature-mobile-tag-management.md
  • ui/mobile/app/(tabs)/_layout.tsx
  • ui/mobile/app/(tabs)/index.tsx
  • ui/mobile/app/(tabs)/search.tsx
  • ui/mobile/app/(tabs)/settings.tsx
  • ui/mobile/app/(tabs)/tags.tsx
  • ui/mobile/components/search/SearchResultsList.tsx
  • ui/mobile/components/settings/AIIndexPanel.tsx
  • ui/mobile/components/tags/AlphabeticalIndex.tsx
  • ui/mobile/components/tags/TagManagementCard.tsx
  • ui/mobile/components/tags/TagSearchInput.tsx
  • ui/mobile/components/tags/index.tsx
  • ui/mobile/eslint.config.mjs
  • ui/mobile/hooks/index.ts
  • ui/mobile/hooks/useTagManagement.ts
  • ui/mobile/hooks/useTagManagementMutations.ts
  • ui/mobile/providers/CollapsibleTabBarProvider.tsx
  • ui/mobile/providers/index.ts
  • ui/mobile/services/sync.ts
  • ui/mobile/tests/component/aiIndexPanel.test.tsx
  • ui/mobile/tests/component/alphabeticalIndex.test.tsx
  • ui/mobile/tests/component/collapsibleTabBarProvider.test.tsx
  • ui/mobile/tests/component/tagSearchInput.test.tsx
  • ui/mobile/tests/component/useTagManagement.test.tsx
  • ui/mobile/tests/component/useTagManagementMutations.test.tsx
  • ui/mobile/tests/integration/bulkSelection.test.tsx
  • ui/mobile/tests/integration/notesScreen.test.tsx
  • ui/mobile/tests/integration/searchScreen.test.tsx
  • ui/mobile/tests/integration/searchScreenAI.test.tsx
  • ui/mobile/tests/integration/settingsScreen.test.tsx
  • ui/mobile/tests/integration/tagsScreen.test.tsx
  • ui/mobile/tests/tsconfig.json
  • ui/mobile/tests/unit/collapsibleTabBar.test.ts
  • ui/mobile/tests/unit/syncService.test.ts
  • ui/mobile/tests/unit/tagManagement.test.ts
  • ui/mobile/tsconfig.json
  • ui/mobile/types/lucide-react-native.d.ts
  • ui/mobile/utils/collapsibleTabBar.ts
  • ui/mobile/utils/tagManagement.ts

Comment thread core/services/notes.ts
Comment thread docs/ai/implementation/feature-mobile-tag-management.md Outdated
Comment thread docs/ai/planning/feature-mobile-tag-management.md Outdated
Comment thread ui/mobile/hooks/useTagManagement.ts
Comment thread ui/mobile/hooks/useTagManagementMutations.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
ui/mobile/services/sync.ts (1)

92-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add an exhaustiveness check to performSync's switch.

The switch on item.operation has no default case. If a new operation value is added to the MutationQueueItem['operation'] union later but this dispatcher isn't updated, performSync would resolve without syncing or throwing, and OfflineSyncManager would treat that as a successful sync and remove the item from the queue, silently dropping the mutation.

Add a default branch that throws, using a never check for compile-time exhaustiveness.

♻️ Proposed fix
 const performSync = async (item: MutationQueueItem, noteService: NoteService): Promise<void> => {
     switch (item.operation) {
         case 'create':
             await syncCreate(item, noteService)
             return
         case 'update':
             await syncUpdate(item, noteService)
             return
         case 'renameTag':
         case 'deleteTag':
             await syncTagMutation(item, noteService)
             return
         case 'delete':
             await noteService.deleteNote(item.noteId)
             return
+        default: {
+            const exhaustiveCheck: never = item.operation
+            throw new Error(`Unhandled mutation operation: ${exhaustiveCheck}`)
+        }
     }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/mobile/services/sync.ts` around lines 92 - 108, Update the switch in
performSync to add a default branch that passes item.operation to a never-typed
exhaustiveness check and throws for unsupported operations, ensuring unhandled
MutationQueueItem operation values cannot resolve as successful syncs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/services/offlineSyncManager.ts`:
- Around line 136-145: Wrap the new this.queue.removeItems(discardedIds) call in
drainQueue with a try/catch, matching the failure-tolerant cleanup pattern
already used in processSyncItem. Log cleanup failures with console.warn and
allow the surrounding drainQueue flow to continue into the subsequent online
sync loop.

In `@core/utils/compactQueue.ts`:
- Around line 42-73: Update withPendingStatus to reset both lastError and
attempts when changing an item to pending, while preserving the existing status
reset. Ensure all callers, including the tag-mutation bypass and
create-consolidation branch in compactNoteOperations, produce a fresh retry
state without stale failure details.

In `@docs/ai/testing/feature-mobile-tag-management.md`:
- Around line 62-63: Update the testing evidence for tags.tsx to resolve the
92.3% changed-file coverage gap: either add tests covering the uncovered
executable lines or explicitly document those remaining lines and gaps in this
feature document before marking the evidence complete. Keep the reported
coverage and test results accurate.

In `@ui/mobile/hooks/useTagManagementMutations.ts`:
- Around line 23-45: The isRetryableBulkMutationError check must recognize
transient PostgrestError SQLSTATE codes, not only numeric HTTP statuses and Node
network codes. Update its string-code handling to classify connection/sqlclient
failures using the appropriate transient prefix such as 08, and include other
established transient PostgREST codes as needed while preserving the existing
checks.

In `@ui/mobile/services/database.ts`:
- Around line 170-189: Update saveNotesInTransaction to provide non-null
timestamp values before the notes INSERT: use a safe fallback for
note.created_at and fall back from note.updated_at to the resolved created_at
value when missing. Pass these resolved values to db.runAsync while preserving
the existing transaction and note-processing behavior.

---

Nitpick comments:
In `@ui/mobile/services/sync.ts`:
- Around line 92-108: Update the switch in performSync to add a default branch
that passes item.operation to a never-typed exhaustiveness check and throws for
unsupported operations, ensuring unhandled MutationQueueItem operation values
cannot resolve as successful syncs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 09234b70-f766-47df-89a6-eecb58b807de

📥 Commits

Reviewing files that changed from the base of the PR and between 498621b and 63725fd.

📒 Files selected for processing (22)
  • core/services/notes.ts
  • core/services/offlineSyncManager.ts
  • core/tests/unit/offline-sync-manager.test.ts
  • core/utils/compactQueue.ts
  • docs/ai/design/feature-mobile-tag-management.md
  • docs/ai/implementation/feature-mobile-tag-management.md
  • docs/ai/planning/feature-mobile-tag-management.md
  • docs/ai/testing/feature-mobile-tag-management.md
  • ui/mobile/app/(tabs)/_layout.tsx
  • ui/mobile/app/(tabs)/tags.tsx
  • ui/mobile/components/tags/AlphabeticalIndex.tsx
  • ui/mobile/components/tags/TagManagementCard.tsx
  • ui/mobile/components/tags/TagSearchInput.tsx
  • ui/mobile/hooks/useTagManagement.ts
  • ui/mobile/hooks/useTagManagementMutations.ts
  • ui/mobile/providers/CollapsibleTabBarProvider.tsx
  • ui/mobile/services/database.ts
  • ui/mobile/services/sync.ts
  • ui/mobile/tests/component/useTagManagement.test.tsx
  • ui/mobile/tests/component/useTagManagementMutations.test.tsx
  • ui/mobile/tests/unit/databaseService.test.ts
  • ui/mobile/tests/unit/syncService.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
  • docs/ai/planning/feature-mobile-tag-management.md
  • ui/mobile/app/(tabs)/_layout.tsx
  • ui/mobile/hooks/useTagManagement.ts
  • docs/ai/implementation/feature-mobile-tag-management.md
  • ui/mobile/tests/unit/syncService.test.ts
  • ui/mobile/providers/CollapsibleTabBarProvider.tsx
  • core/services/notes.ts
  • ui/mobile/tests/component/useTagManagement.test.tsx
  • docs/ai/design/feature-mobile-tag-management.md

Comment thread core/services/offlineSyncManager.ts
Comment thread core/utils/compactQueue.ts
Comment thread docs/ai/testing/feature-mobile-tag-management.md
Comment thread ui/mobile/hooks/useTagManagementMutations.ts
Comment thread ui/mobile/services/database.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/tests/unit/offline-sync-additional-branches.test.ts`:
- Around line 144-165: Update the “continues draining when compacted queue
cleanup fails” test fixture so DeterministicStorage.upsertQueue preserves
existing queue items and applies item-level upsert semantics instead of
replacing the entire queue. Strengthen the assertions to verify only the latest
item is passed to performSync, the superseded stale item is never synchronized,
and getPendingBatch is called the expected number of times rather than merely
once.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 94ab07c8-bf92-48d6-828d-ad5d88122417

📥 Commits

Reviewing files that changed from the base of the PR and between 63725fd and 260370d.

📒 Files selected for processing (11)
  • AGENTS.md
  • core/services/offlineSyncManager.ts
  • core/tests/unit/compact-queue-additional-branches.test.ts
  • core/tests/unit/offline-sync-additional-branches.test.ts
  • core/tests/unit/tag-mutation-queue.test.ts
  • core/utils/compactQueue.ts
  • docs/ai/testing/feature-mobile-tag-management.md
  • ui/mobile/hooks/useTagManagementMutations.ts
  • ui/mobile/services/database.ts
  • ui/mobile/tests/component/useTagManagementMutations.test.tsx
  • ui/mobile/tests/unit/databaseService.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • ui/mobile/tests/unit/databaseService.test.ts
  • ui/mobile/hooks/useTagManagementMutations.ts
  • core/services/offlineSyncManager.ts
  • core/utils/compactQueue.ts
  • ui/mobile/services/database.ts

Comment on lines +144 to +165
it("continues draining when compacted queue cleanup fails", async () => {
const storage = new DeterministicStorage()
storage.queue = [
makeItem("stale", {
noteId: "same-note",
clientUpdatedAt: "2026-01-01T00:00:01Z",
}),
makeItem("latest", {
noteId: "same-note",
clientUpdatedAt: "2026-01-01T00:00:02Z",
}),
]
storage.removeQueueItems.mockRejectedValueOnce(new Error("cleanup failed"))
const warn = jest.spyOn(console, "warn").mockImplementation(() => undefined)
const performSync = jest.fn().mockResolvedValue(undefined)
const manager = new OfflineSyncManager(storage, performSync, makeNetwork(false))

await expect(manager.handleOnline()).resolves.toBeUndefined()

expect(warn).toHaveBeenCalledWith("Failed to remove compacted queue items:", expect.any(Error))
expect(performSync).toHaveBeenCalledWith(expect.objectContaining({ id: "latest" }))
expect(storage.getPendingBatch).toHaveBeenCalled()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make this cleanup-failure test model the queue contract.

OfflineSyncManager.drainQueue() uses item-level upsertQueue and then removes discarded IDs separately. However, DeterministicStorage.upsertQueue at Lines 24-26 replaces the entire queue. When removeQueueItems fails at Line 156, stale is already gone, so this test cannot detect replay of the superseded item with the real adapter behavior.

The assertions are also too weak. toHaveBeenCalled() does not prove a second pending-batch read, and toHaveBeenCalledWith(...) does not prove that stale was not synchronized. Use item-level upsert semantics in the fixture, assert that only latest is synchronized, and assert the expected batch-read count.

Proposed test hardening
-  readonly upsertQueue = jest.fn(async (items: MutationQueueItem[]) => {
-    this.queue = [...items]
-  })
+  readonly upsertQueue = jest.fn(async (items: MutationQueueItem[]) => {
+    for (const item of items) {
+      const existingIndex = this.queue.findIndex((entry) => entry.id === item.id)
+      if (existingIndex === -1) {
+        this.queue.push(item)
+      } else {
+        this.queue[existingIndex] = item
+      }
+    }
+  })

-    expect(performSync).toHaveBeenCalledWith(expect.objectContaining({ id: "latest" }))
-    expect(storage.getPendingBatch).toHaveBeenCalled()
+    expect(performSync).toHaveBeenCalledTimes(1)
+    expect(performSync).toHaveBeenCalledWith(expect.objectContaining({ id: "latest" }))
+    expect(performSync).not.toHaveBeenCalledWith(expect.objectContaining({ id: "stale" }))
+    expect(storage.getPendingBatch).toHaveBeenCalledTimes(2)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/tests/unit/offline-sync-additional-branches.test.ts` around lines 144 -
165, Update the “continues draining when compacted queue cleanup fails” test
fixture so DeterministicStorage.upsertQueue preserves existing queue items and
applies item-level upsert semantics instead of replacing the entire queue.
Strengthen the assertions to verify only the latest item is passed to
performSync, the superseded stale item is never synchronized, and
getPendingBatch is called the expected number of times rather than merely once.

denyskoreiba and others added 3 commits August 20, 2026 20:25
When removing compaction-discarded rows failed, item-level adapters kept
the superseded row pending; syncing its superseding item anyway left the
stale row free to replay in a later drain and silently revert newer
edits. Hold back every note touched by an unremoved superseded row for
the current drain (unaffected notes keep syncing) and let the next drain
re-compact and retry the removal.

The cleanup-failure test now mirrors the real adapters' item-level
upsert semantics and asserts the full contract, addressing the remaining
CodeRabbit review finding.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The workflow never set APP_VARIANT and exported only NEXT_PUBLIC_* web
vars with placeholder fallbacks, so Expo resolved the dev variant with
an empty Supabase config during the Gradle build and the release APK
crashed on startup.

- select the variant via APP_VARIANT/EXPO_PUBLIC_APP_VARIANT
- feed variant-specific EXPO_PUBLIC_SUPABASE_* from secrets (stage falls
  back to the existing NEXT_PUBLIC_* web secrets), plus the stage editor
  WebView URL and prod public web origin
- derive the webview-bundle NEXT_PUBLIC_* from the same target so the
  bundled editor and the native side always share one Supabase project
- enable the test-login button for stage builds
- fail fast: validate inputs, probe Supabase /auth/v1/health, and verify
  the resolved Expo config before Gradle runs

Documented the env contract in docs/ai/deployment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… builds

The verification metadata was generated on Windows, so the macOS aapt2
artifact and several junit-bom Gradle module-metadata files were missing
and local builds failed verification. Hashes verified against
repo1.maven.org and dl.google.com.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

New Tests in Allure Report Allure Report

Show 1 new test

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@koreyba koreyba closed this Sep 15, 2026

This branch was previously deployed

1 inactive deployment
Stage — a60de0b3 Deployed Aug 20, 2026 by koreyba via build #930
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.

2 participants