apps/desktop/src/components/inspector/PageEditor.tsx (1)

61-66: Fix first-page detection: compare IDs, not object refs.

Object identity is brittle; different instances representing the same page will fail this check, causing incorrect disabling and display.

-        if (pages.length > 0) {
-            const firstPage = pages[0];
-            setIsFirstPage(selectedPage === firstPage);
-        }
+        if (pages.length > 0) {
+            const firstPage = pages[0];
+            setIsFirstPage(!!selectedPage && selectedPage.id === firstPage.id);
+        }

apps/desktop/i18n/es.json (2)

196-199: Placeholder name mismatch: use {status}, not {estado}.

This will throw at runtime or during i18n compile/validation.

Apply this diff:

-        "marcher": "Marcher: {estado}",
+        "marcher": "Marcher: {status}",

---

1-756: Add missing export keys and fix ICU/placeholder issues in es.json

• Missing keys — add exportCoordinates.marchersNotLoaded and exportCoordinates.marcherPagesNotLoaded to apps/desktop/i18n/es.json (these are present in en.json).
• Invalid ICU plural — apps/desktop/i18n/es.json:27 (setSelectedToPreviousSuccess) contains a malformed double-brace plural ("{{count, plural...}"); fix to a valid ICU plural pattern ({count, plural, one {…} other {…}}). Also check apps/desktop/i18n/pt-BR.json:27.
• Placeholder name mismatch — apps/desktop/i18n/es.json:197 ("marcher": "Marcher: {estado}") uses {estado}; rename to the placeholder used in en.json (e.g., {status}) so runtime interpolation matches.

apps/desktop/i18n/en.json (1)

1-762: Fix ICU plural syntax and placeholder name mismatch in i18n locales

• Invalid ICU plural syntax (extra "{") in apps/desktop/i18n/es.json and apps/desktop/i18n/pt-BR.json at actions.batchEdit.setSelectedToPreviousSuccess — contains "{{count, plural,...}}". Replace the double "{{" with a single "{" so it's "{count, plural,...}".
• Placeholder mismatch: apps/desktop/i18n/es.json exportCoordinates.marcher uses {estado} while en.json (and other locales) use {status}. Rename {estado} → {status} so placeholder names are consistent.

apps/desktop/i18n/pt-BR.json (1)

1-756: Add missing exportCoordinates keys and fix ICU plural formatting

• Add exportCoordinates.marchersNotLoaded and exportCoordinates.marcherPagesNotLoaded to apps/desktop/i18n/pt-BR.json to match en.json.
• Fix the invalid double-brace ICU plural occurrences at:
  • apps/desktop/i18n/pt-BR.json — actions.batchEdit.setSelectedToPreviousSuccess (line ~27)
  • apps/desktop/i18n/es.json — actions.batchEdit.setSelectedToPreviousSuccess (line ~27)
    Replace the malformed "{{count, plural…}}" insertion with a proper ICU plural block.

apps/desktop/src/components/marcher/NewMarcherForm.tsx (3)

80-112: Guard against NaN drillOrder; prevents invalid insert

If the user clears the drill order field, parseInt('') yields NaN. That flows into Set/loop math and can reach the DB as NaN (violates integer schema).

Apply this diff to short‑circuit invalid input before building payloads:

     const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
-        setContent(<MarcherListContents />, "marchers");
         event.preventDefault();
+        // Navigate after we know we submitted (optional)
+        setContent(<MarcherListContents />, "marchers");
+        if (!Number.isFinite(drillOrder)) {
+            setDrillOrderError(t("marchers.drillOrderError.invalid"));
+            return;
+        }
         let newDrillOrderOffset = 0;
         const existingDrillOrders = new Set<number>(
             (marchers || [])
                 .filter(
                     (marcher: Marcher) => marcher.drill_prefix === drillPrefix,
                 )
                 .map((marcher: Marcher) => marcher.drill_order),
         );

Additionally, update validation to treat non‑numeric values as invalid (see separate comment below).

---

138-154: Validation gap: treat non-numeric drill orders as errors

Without this, NaN passes validation and enables submit.

     const validateDrillOrder = useCallback(
         (drillOrder: number) => {
+            if (!Number.isFinite(drillOrder)) {
+                setDrillOrderError(t("marchers.drillOrderError.required"));
+                return;
+            }
             const existingMarchers = (marchers || []).filter(
                 (marcher: Marcher) => marcher.drill_prefix === drillPrefix,
             );

Also consider disabling submit if drillOrder is not finite (see setSubmitIsDisabled effect).

---

85-90: Don’t store translated section labels in the DB

On first load you set section to a translated label (defaultSection(t)), but you persist section as-is in the payload. If the user never changes the section, you’ll store localized strings (e.g., “Otro”) instead of canonical keys (“Other”).

Recommended approach: keep state as canonical section keys; translate only for display. Example changes (outside the exact ranges shown):

// near line 33
const DEFAULT_SECTION_KEY = "Other";

// lines 64 and 68
setSection(DEFAULT_SECTION_KEY);

// line 245 (label)
<SelectTriggerButton label={getTranslatedSectionName(section || DEFAULT_SECTION_KEY, t)} />

// line 206 (submit disabled)
section === DEFAULT_SECTION_KEY || section === undefined

// payload (lines 103-107)
newMarchers.push({
  section: section || DEFAULT_SECTION_KEY,
  drill_prefix: drillPrefix,
  drill_order: newDrillOrder,
});

Also applies to: 103-107, 63-69

apps/desktop/electron/main/services/export-utility-process.ts (1)

5-5: Fix build blockers: remove unused import/variable.

path is imported but never used; organizeBySection is destructured but unused. This can fail builds with noUnusedLocals.

-import path from "path";
...
-    const { dbPath, filePath, quarterPages, organizeBySection, ...options } =
-        payload;
+    const { dbPath, filePath, quarterPages, ...options } = payload;

Also applies to: 156-158

apps/desktop/src/components/marcher/__test__/NewMarcherForm.test.tsx (1)

55-65: Fix tests that mock a removed Marcher store

• Tests and mocks still import/vi.mock "@/stores/MarcherStore" and "@/stores/marcher/useMarcherStore" but there is no Marcher store implementation under apps/desktop/src/stores — this will break test compilation. Affected files: apps/desktop/src/components/marcher/test/NewMarcherForm.test.tsx, apps/desktop/src/components/marcher/test/MarcherList.test.tsx, apps/desktop/src/mocks/components/ChangeMarchersSelection.mock.tsx.
• Replace these mocks with the new React Query hooks (mock the query/mutation hooks) and wrap affected tests with a QueryClientProvider — or restore/fix the module path before re‑enabling the suites.

apps/desktop/electron/database/tables/__test__/testUtils.ts (1)

1-61: Move test helper out of legacy electron/database/tables/test

This path is legacy — tests/utilities under apps/desktop/electron/database/tables are being removed. Move apps/desktop/electron/database/tables/test/testUtils.ts to apps/desktop/src/global/classes/test/ or a shared test-utils module and update all imports accordingly. Can draft a follow-up PR to move the file and update imports.

apps/desktop/electron/main/index.ts (2)

585-615: Guard dialog cancellation before accessing filePaths[0]

Accessing path.filePaths[0] when the dialog is canceled throws. The check is currently commented out.

 .then(async (path) => {
-    // If the user cancels the dialog, and there is no previous path, return -1
-    // if (path.canceled || !path.filePaths[0]) return -1;
+    if (path.canceled || !path.filePaths[0]) return -1;
 
     DatabaseServices.setDbPath(path.filePaths[0]);

---

709-737: Fix order: check cancellation before reading file

readFileSync(path.filePaths[0]) runs even when the dialog is canceled.

 .then(async (path) => {
-    const fileContents = fs.readFileSync(path.filePaths[0]);
-    const jsonStr = fileContents.toString();
-    await updateFieldsPropertiesJSON(jsonStr);
-
-    // If the user cancels the dialog, and there is no previous path, return -1
-    if (path.canceled || !path.filePaths[0]) return -1;
+    if (path.canceled || !path.filePaths[0]) return -1;
+    const fileContents = fs.readFileSync(path.filePaths[0]);
+    const jsonStr = fileContents.toString();
+    await updateFieldsPropertiesJSON(jsonStr);
 
     win?.webContents.send("field_properties:onImport");

apps/desktop/electron/database/database.services.ts (2)

146-184: Implement the "values" method for the Drizzle proxy

Drizzle can call the "values" method. Not handling it will throw at runtime.

     switch (method) {
       case "all":
         rows = result.all(...params);
         return {
           rows: rows
             ? rows.map((row: { [key: string]: any }) => Object.values(row))
             : [],
         };
       case "get":
         rows = result.get(...params);
         return {
           rows: rows ? Object.values(rows as Record<string, any>) : undefined,
         };
+      case "values":
+        rows = result.all(...params);
+        return {
+          rows: rows
+            ? rows.map((row: { [key: string]: any }) => Object.values(row))
+            : [],
+        };
       case "run":
         rows = result.run(...params);
         return { rows: [] };
       default:
         throw new Error(`Unknown method: ${method}`);
     }

---

245-264: Gate the unsafe SQL IPC in non‑production builds

Exposing arbitrary exec() to the renderer increases risk if the renderer is compromised. Restrict to test/dev via an env flag.

 export function initHandlers() {
   // Generic SQL proxy handler for Drizzle ORM
   ipcMain.handle("sql:proxy", handleSqlProxy);
-  ipcMain.handle("unsafeSql:proxy", handleUnsafeSqlProxy);
+  if (process.env.ENABLE_UNSAFE_SQL_PROXY === "1") {
+    ipcMain.handle("unsafeSql:proxy", handleUnsafeSqlProxy);
+  }

Also ensure preload only calls unsafeSqlProxy when the flag is enabled.

apps/desktop/electron/preload/index.ts (1)

220-243: History IPC handlers missing — implement handlers or remove exposed APIs

Preload exposes history:* methods (apps/desktop/electron/preload/index.ts lines 220-243) but no ipcMain handlers for history:* were found; these ipcRenderer invocations will reject at runtime. Add matching ipcMain.handle/on implementations in the main/database service or remove these methods from preload.

apps/desktop/electron/database/migrations/schema.ts (2)

265-271: Use browser-safe blob type for audio_files.data

blob() maps to Buffer in Drizzle; Buffer isn’t available in the browser/SQL.js. You already defined browserSafeBinaryBlob; use it here to avoid web build issues.

Apply:

 export const audio_files = sqliteTable("audio_files", {
   id: integer().primaryKey(),
   path: text().notNull(),
   nickname: text(),
-  data: blob(),
+  data: browserSafeBinaryBlob(),
   selected: integer().default(0).notNull(),

Note: If this is the last use of blob(), remove blob from the sqlite-core import to avoid unused import warnings.

---

316-333: Schema/migration drift — make position_order NOT NULL in the migration

apps/desktop/electron/database/migrations/schema.ts (lines 316–333) declares shape_page_marchers.position_order as .notNull(), but apps/desktop/electron/database/migrations/0007_misty_zemo.sql (CREATE TABLE __new_shape_page_marchers around lines ~135–153) defines position_order as nullable; this allows multiple NULLs despite the UNIQUE (shape_page_id, position_order) index. Update 0007 (and any other SQL that recreates this table) to declare position_order INTEGER NOT NULL and backfill existing NULLs (e.g., assign sequential position_order per shape_page_id) before enforcing NOT NULL — or, if NULL is intended, remove .notNull() from schema.ts.

apps/desktop/electron/database/migrations/schema.ts (2)

186-193: Allow NULL path positions for marcher pages

path_start_position/path_end_position are declared nullable and the docstring says NULL should be interpreted as 0/1, but this CHECK rejects NULL because SQLite treats NULL as constraint failure. Inserts will blow up whenever we leave these optional fields unset. Please relax the CHECK to tolerate NULL while still enforcing the 0‑1 bound when a value is provided, and regenerate the snapshot/migration after the change.

         check(
             "marcher_pages_path_data_position_check",
-            sql`path_start_position >= 0 AND path_start_position <= 1 AND path_end_position >= 0 AND path_end_position <= 1`,
+            sql`(path_start_position IS NULL OR (path_start_position >= 0 AND path_start_position <= 1)) AND (path_end_position IS NULL OR (path_end_position >= 0 AND path_end_position <= 1))`,
         ),

---

217-224: Allow NULL path positions for midsets

Same issue here: the CHECK currently bars NULL even though both columns are optional. This will cause failures when midsets don’t specify a path. Please apply the same NULL-tolerant guard as above.

         check(
             "midsets_path_data_position_check",
-            sql`path_start_position >= 0 AND path_start_position <= 1 AND path_end_position >= 0 AND path_end_position <= 1`,
+            sql`(path_start_position IS NULL OR (path_start_position >= 0 AND path_start_position <= 1)) AND (path_end_position IS NULL OR (path_end_position >= 0 AND path_end_position <= 1))`,
         ),