feat(tree,collections,firestore): tree field add/delete and admin timestamp fix

electron/controllers/firestoreController.js

@@ -8,7 +8,7 @@ const fs = require('fs');
 const vm = require('vm');
 const { FieldValue, Filter, Timestamp, GeoPoint } = require('firebase-admin/firestore');
 const { fetchDocumentsPage } = require('./firestore/documentList');
-const { firestoreDocumentToData } = require('../utils/firestoreHelpers');
+const { firestoreDocumentToData, toFirestoreAdminValue } = require('../utils/firestoreHelpers');
 
 let dbRef = null;
 
@@ -101,7 +101,7 @@ function registerHandlers() {
       const docRef = documentId
         ? dbRef.collection(collectionPath).doc(documentId)
         : dbRef.collection(collectionPath).doc();
-      await docRef.set(data);
+      await docRef.set(toFirestoreAdminValue(data, { Timestamp, GeoPoint, FieldValue }));
       return { success: true, documentId: docRef.id };
     } catch (error) {
       return { success: false, error: error.message };
@@ -112,7 +112,7 @@ function registerHandlers() {
   ipcMain.handle('firestore:updateDocument', async (event, { documentPath, data }) => {
     try {
       if (!dbRef) throw new Error('Not connected to Firebase');
-      await dbRef.doc(documentPath).update(data);
+      await dbRef.doc(documentPath).update(toFirestoreAdminValue(data, { Timestamp, GeoPoint, FieldValue }));
       return { success: true };
     } catch (error) {
       return { success: false, error: error.message };
@@ -123,7 +123,7 @@ function registerHandlers() {
   ipcMain.handle('firestore:setDocument', async (event, { documentPath, data }) => {
     try {
       if (!dbRef) throw new Error('Not connected to Firebase');
-      await dbRef.doc(documentPath).set(data);
+      await dbRef.doc(documentPath).set(toFirestoreAdminValue(data, { Timestamp, GeoPoint, FieldValue }));
       return { success: true };
     } catch (error) {
       return { success: false, error: error.message };
@@ -212,7 +212,7 @@ function registerHandlers() {
 
         for (const [docId, docData] of Object.entries(data)) {
           const docRef = dbRef.collection(collectionPath).doc(docId);
-          batch.set(docRef, docData);
+          batch.set(docRef, toFirestoreAdminValue(docData, { Timestamp, GeoPoint, FieldValue }));
           count++;
           if (count >= 500) {
             await batch.commit();

electron/controllers/firestoreController.test.js

@@ -36,11 +36,21 @@ require_.cache[require_.resolve('firebase-admin/firestore')] = {
   exports: {
     FieldValue: { serverTimestamp: vi.fn() },
     Filter: { where: vi.fn() },
-    Timestamp: { now: vi.fn() },
-    GeoPoint: vi.fn(),
+    Timestamp: vi.fn(function (seconds, nanoseconds) {
+      this.seconds = seconds;
+      this.nanoseconds = nanoseconds;
+    }),
+    GeoPoint: vi.fn(function (latitude, longitude) {
+      this.latitude = latitude;
+      this.longitude = longitude;
+    }),
   },
 };
 
+const firestoreMock = require_('firebase-admin/firestore');
+const electronMock = require_('electron');
+const fsMock = require_('fs');
+
 // Load controller with mocked deps
 const controllerPath = require_.resolve('./firestoreController');
 delete require_.cache[controllerPath];
@@ -295,4 +305,92 @@ describe('firestoreController', () => {
     expect(result.success).toBe(false);
     expect(result.error).toContain('Not connected');
   });
+
+  // ─── admin write paths convert app-shaped values ─────────────────────────
+
+  const appShapedData = () => ({
+    name: 'test',
+    count: 3,
+    active: true,
+    createdAt: { _seconds: 1767225600, _nanoseconds: 500 },
+    location: { _latitude: 1.5, _longitude: -2.5 },
+    tags: [{ _seconds: 10, _nanoseconds: 0 }, 'plain'],
+    meta: { nested: { _seconds: 20, _nanoseconds: 1 } },
+  });
+
+  const convertedData = () => ({
+    name: 'test',
+    count: 3,
+    active: true,
+    createdAt: { seconds: 1767225600, nanoseconds: 500 },
+    location: { latitude: 1.5, longitude: -2.5 },
+    tags: [{ seconds: 10, nanoseconds: 0 }, 'plain'],
+    meta: { nested: { seconds: 20, nanoseconds: 1 } },
+  });
+
+  it('setDocument converts timestamps and geopoints before writing', async () => {
+    const set = vi.fn().mockResolvedValue(undefined);
+    setRefs(null, { doc: vi.fn().mockReturnValue({ set }) });
+    const data = appShapedData();
+
+    const result = await handlers['firestore:setDocument'](null, { documentPath: 'col/doc', data });
+
+    expect(result.success).toBe(true);
+    expect(firestoreMock.Timestamp).toHaveBeenCalled();
+    expect(firestoreMock.GeoPoint).toHaveBeenCalled();
+    expect(set.mock.calls[0][0]).toEqual(convertedData());
+    // The incoming payload must not be mutated.
+    expect(data.createdAt).toEqual({ _seconds: 1767225600, _nanoseconds: 500 });
+    expect(data.location).toEqual({ _latitude: 1.5, _longitude: -2.5 });
+  });
+
+  it('updateDocument converts timestamps and geopoints before writing', async () => {
+    const update = vi.fn().mockResolvedValue(undefined);
+    setRefs(null, { doc: vi.fn().mockReturnValue({ update }) });
+    const data = appShapedData();
+
+    const result = await handlers['firestore:updateDocument'](null, { documentPath: 'col/doc', data });
+
+    expect(result.success).toBe(true);
+    expect(update.mock.calls[0][0]).toEqual(convertedData());
+    expect(data.createdAt).toEqual({ _seconds: 1767225600, _nanoseconds: 500 });
+  });
+
+  it('createDocument converts timestamps and geopoints before writing', async () => {
+    const set = vi.fn().mockResolvedValue(undefined);
+    const docRef = { id: 'new-id', set };
+    const mockCollection = { doc: vi.fn().mockReturnValue(docRef) };
+    setRefs(null, { collection: vi.fn().mockReturnValue(mockCollection) });
+    const data = appShapedData();
+
+    const result = await handlers['firestore:createDocument'](null, {
+      collectionPath: 'col',
+      documentId: 'new-id',
+      data,
+    });
+
+    expect(result.success).toBe(true);
+    expect(result.documentId).toBe('new-id');
+    expect(set.mock.calls[0][0]).toEqual(convertedData());
+  });
+
+  it('importDocuments converts timestamps and geopoints for each batch set', async () => {
+    const batchSet = vi.fn();
+    const commit = vi.fn().mockResolvedValue(undefined);
+    const doc = vi.fn().mockReturnValue({});
+    setRefs(null, {
+      collection: vi.fn().mockReturnValue({ doc }),
+      batch: vi.fn().mockReturnValue({ set: batchSet, commit }),
+    });
+    electronMock.dialog.showOpenDialog.mockResolvedValue({ filePaths: ['/tmp/import.json'] });
+    fsMock.readFileSync.mockReturnValue(JSON.stringify({ doc1: appShapedData(), doc2: { plain: 'value' } }));
+
+    const result = await handlers['firestore:importDocuments'](null, 'col');
+
+    expect(result.success).toBe(true);
+    expect(result.count).toBe(2);
+    expect(batchSet.mock.calls[0][1]).toEqual(convertedData());
+    expect(batchSet.mock.calls[1][1]).toEqual({ plain: 'value' });
+    expect(commit).toHaveBeenCalled();
+  });
 });

electron/utils/firestoreHelpers.js

@@ -42,6 +42,57 @@ function convertToFirestoreValue(value) {
   return { stringValue: String(value) };
 }
 
+/**
+ * Converts app-shaped data into firebase-admin compatible values.
+ * The UI works with `{ _seconds, _nanoseconds }` timestamps and
+ * `{ _latitude, _longitude }` geopoints; the Admin SDK needs real
+ * Timestamp/GeoPoint instances, otherwise it persists them as plain maps.
+ * Constructors are injected so this module stays free of firebase-admin.
+ * @param {*} value - App-shaped value to convert
+ * @param {Object} opts - { Timestamp, GeoPoint, FieldValue } constructors
+ * @returns {*} - Value safe to hand to the firebase-admin SDK
+ */
+function toFirestoreAdminValue(value, { Timestamp, GeoPoint, FieldValue } = {}) {
+  if (value === null || value === undefined) return value;
+  if (value instanceof Date) {
+    const time = value.getTime();
+    return new Timestamp(Math.floor(time / 1000), (time % 1000) * 1000000);
+  }
+  if (typeof value !== 'object') return value;
+
+  // Sentinels (serverTimestamp, delete, increment, ...) must pass through.
+  if (typeof FieldValue === 'function' && value instanceof FieldValue) return value;
+
+  if (Array.isArray(value)) {
+    return value.map((item) => toFirestoreAdminValue(item, { Timestamp, GeoPoint, FieldValue }));
+  }
+
+  // Timestamp shapes: { _seconds, _nanoseconds } or { seconds, nanoseconds }.
+  const nanos = value._nanoseconds ?? value.nanoseconds ?? value.nanos;
+  const underscoredSeconds = typeof value._seconds === 'number' ? value._seconds : undefined;
+  const plainSeconds = typeof value.seconds === 'number' ? value.seconds : undefined;
+  if (underscoredSeconds !== undefined || (plainSeconds !== undefined && nanos !== undefined)) {
+    const seconds = underscoredSeconds !== undefined ? underscoredSeconds : plainSeconds;
+    return new Timestamp(seconds, nanos || 0);
+  }
+
+  // GeoPoint shapes: { _latitude, _longitude } or { latitude, longitude }.
+  const underscoredLatLng = value._latitude !== undefined && value._longitude !== undefined;
+  const plainLatLng = value.latitude !== undefined && value.longitude !== undefined;
+  if (underscoredLatLng || plainLatLng) {
+    const latitude = underscoredLatLng ? value._latitude : value.latitude;
+    const longitude = underscoredLatLng ? value._longitude : value.longitude;
+    return new GeoPoint(latitude, longitude);
+  }
+
+  // Plain map: rebuild recursively, never mutate the input.
+  const result = {};
+  for (const [key, item] of Object.entries(value)) {
+    result[key] = toFirestoreAdminValue(item, { Timestamp, GeoPoint, FieldValue });
+  }
+  return result;
+}
+
 /**
  * Parses a Firestore REST API value to JS value
  * @param {Object} value - Firestore REST API value object
@@ -118,6 +169,7 @@ function firestoreDocumentToData(doc) {
 
 module.exports = {
   convertToFirestoreValue,
+  toFirestoreAdminValue,
   parseFirestoreValue,
   parseFirestoreDocument,
   dataToFirestoreFields,

electron/utils/firestoreHelpers.test.js

@@ -0,0 +1,112 @@
+// @vitest-environment node
+import { describe, it, expect, beforeEach } from 'vitest';
+import { createRequire } from 'module';
+
+const require_ = createRequire(import.meta.url);
+const { toFirestoreAdminValue } = require_('./firestoreHelpers');
+
+class FakeTimestamp {
+  constructor(seconds, nanoseconds) {
+    this.seconds = seconds;
+    this.nanoseconds = nanoseconds;
+  }
+}
+
+class FakeGeoPoint {
+  constructor(latitude, longitude) {
+    this.latitude = latitude;
+    this.longitude = longitude;
+  }
+}
+
+class FakeFieldValue {}
+
+const opts = () => ({ Timestamp: FakeTimestamp, GeoPoint: FakeGeoPoint, FieldValue: FakeFieldValue });
+
+describe('toFirestoreAdminValue', () => {
+  it('converts a Date into a Timestamp with second and nanosecond parts', () => {
+    const result = toFirestoreAdminValue(new Date('2026-01-01T00:00:00.000Z'), opts());
+
+    expect(result).toBeInstanceOf(FakeTimestamp);
+    expect(result).toEqual({ seconds: 1767225600, nanoseconds: 0 });
+  });
+
+  it('preserves sub-second milliseconds as nanoseconds', () => {
+    const result = toFirestoreAdminValue(new Date(1500), opts());
+
+    expect(result).toEqual({ seconds: 1, nanoseconds: 500000000 });
+  });
+
+  it('converts the { _seconds, _nanoseconds } shape', () => {
+    const result = toFirestoreAdminValue({ _seconds: 1767225600, _nanoseconds: 42 }, opts());
+
+    expect(result).toBeInstanceOf(FakeTimestamp);
+    expect(result).toEqual({ seconds: 1767225600, nanoseconds: 42 });
+  });
+
+  it('converts the { seconds, nanoseconds } shape', () => {
+    const result = toFirestoreAdminValue({ seconds: 1767225600, nanoseconds: 7 }, opts());
+
+    expect(result).toBeInstanceOf(FakeTimestamp);
+    expect(result).toEqual({ seconds: 1767225600, nanoseconds: 7 });
+  });
+
+  it('defaults missing nanos to zero for the seconds shape', () => {
+    const result = toFirestoreAdminValue({ seconds: 10, nanos: 3 }, opts());
+
+    expect(result).toEqual({ seconds: 10, nanoseconds: 3 });
+  });
+
+  it('converts { _latitude, _longitude } geopoints', () => {
+    const result = toFirestoreAdminValue({ _latitude: 1.5, _longitude: -2.5 }, opts());
+
+    expect(result).toBeInstanceOf(FakeGeoPoint);
+    expect(result).toEqual({ latitude: 1.5, longitude: -2.5 });
+  });
+
+  it('converts { latitude, longitude } geopoints', () => {
+    const result = toFirestoreAdminValue({ latitude: 40.7, longitude: -74 }, opts());
+
+    expect(result).toEqual({ latitude: 40.7, longitude: -74 });
+  });
+
+  it('converts values nested inside arrays and plain maps', () => {
+    const input = {
+      tags: [{ _seconds: 1, _nanoseconds: 0 }, 'plain', 5],
+      meta: { place: { _latitude: 1, _longitude: 2 } },
+    };
+
+    const result = toFirestoreAdminValue(input, opts());
+
+    expect(result.tags[0]).toBeInstanceOf(FakeTimestamp);
+    expect(result).toEqual({
+      tags: [{ seconds: 1, nanoseconds: 0 }, 'plain', 5],
+      meta: { place: { latitude: 1, longitude: 2 } },
+    });
+  });
+
+  it('passes primitives through unchanged', () => {
+    expect(toFirestoreAdminValue(null, opts())).toBeNull();
+    expect(toFirestoreAdminValue(undefined, opts())).toBeUndefined();
+    expect(toFirestoreAdminValue('text', opts())).toBe('text');
+    expect(toFirestoreAdminValue(3, opts())).toBe(3);
+    expect(toFirestoreAdminValue(true, opts())).toBe(true);
+  });
+
+  it('returns FieldValue sentinels unchanged', () => {
+    const sentinel = new FakeFieldValue();
+
+    expect(toFirestoreAdminValue(sentinel, opts())).toBe(sentinel);
+  });
+
+  it('does not mutate the input object and rebuilds plain maps', () => {
+    const input = { createdAt: { _seconds: 5, _nanoseconds: 0 }, nested: { a: 1 } };
+
+    const result = toFirestoreAdminValue(input, opts());
+
+    expect(result).not.toBe(input);
+    expect(result.nested).not.toBe(input.nested);
+    expect(input.createdAt).toEqual({ _seconds: 5, _nanoseconds: 0 });
+    expect(input.nested).toEqual({ a: 1 });
+  });
+});

odd/tasks/tree-add-fields.md

@@ -0,0 +1,53 @@
+# Feature: Add fields from Tree view — type-aware
+
+- **Feature id**: `tree-add-fields`
+- **Branch**: `feat/add-fields` (already checked out, base `34923d6`)
+- **Scope**: Documento + Maps anidados (user choice 2026-09-22). No Array indices.
+- **Status**: implemented + verified; native review ESCALATED (terminal stop, see below)
+
+## Objective
+
+Permitir agregar fields en Tree view especificando el tipo de valor, de forma intuitiva y alineada a la convención del proyecto.
+
+## Why
+
+JSON permite agregar pero no es intuitivo; Table solo edita top-level. Tree modela cada field anidado con path `docId.field.nested`.
+
+## Conventions to reuse (English artifacts)
+
+- Types: `getValueType` in `src/shared/utils/firestoreUtils.ts` → String, Integer, Number, Boolean, Null, Timestamp, GeoPoint, Array, Map (+ Reference as string passthrough).
+- Colors: `getTypeColor(type, isDark)`.
+- Dialogs: MUI `Dialog/Title/Content/Actions` like `AddDocumentDialog.tsx` + `EditDialog.tsx` (Ctrl+Enter to save, `MONOSPACE_FONT_FAMILY`, Type badge).
+- Date input: `datetime-local` like `TreeNodeRow.tsx:144` + `DatePopover.tsx`, helpers in `dateUtils.ts` (`formatDateForDateTimeLocal`, `isIsoDateString`).
+- Tree plumbing: `TreeContext.ts` + `TreeNodeRow.tsx` + `TreeView.tsx` + handlers in `CollectionTab.tsx` (`onCellEdit/onCellSave`, `updateDocument` thunk in `collectionSlice.ts`).
+- Service: `documentService.ts` (`transformValueForSave`, `prepareUpdateData` top-level only) — add nested-path variant.
+
+## Tasks
+
+- [x] TD-1 `AddFieldDialog` type-aware component (`src/features/collections/components/tree/AddFieldDialog.tsx`): props `open, parentPathLabel, existingKeys, onClose, onSubmit(fieldName, value)`; field-name TextField with duplicate/empty validation; MUI Select of types String,Integer,Number,Boolean,Null,Timestamp,GeoPoint,Array,Map,Reference; per-type input (TextField / number / Select True-False / datetime-local / lat,lng / JSON multiline for Array,Map with JSON.parse + error / Reference string); default values per type; Ctrl+Enter saves. Reuse `MONOSPACE_FONT_FAMILY`, Type badge style from EditDialog.
+- [x] TD-2 Nested add plumbing: `documentService.prepareAddData(docDataRecord, parentPath, fieldName, value)` (dot-path set, reject duplicates/empty, no Array indices); `TreeContext` += `onAddField(docId, parentPath, docData, docCollectionPath)`; `CollectionTab` implements it (opens dialog, on submit builds new data, dispatches `updateDocument`, refreshes, shows message); `TreeView` forwards through context; `TreeNodeRow` shows hover `Add` IconButton (AddIcon) on Document rows and Map rows → calls `onAddField` with nested path.
+- [x] TD-3 Verification: `pnpm typecheck` clean, `pnpm test` 12 files / 89 passed, scoped lint + format clean (writer + parent spot-check 2026-09-22). TDD off (no sdd-init capabilities on file).
+
+## Acceptance
+
+- Expand doc or Map → hover reveals Add action → dialog pide nombre + tipo + valor adaptado → Save crea el field y refresca el tree.
+- Nombre vacío/duplicado/puntos bloquea con helper error; JSON inválido y fecha inválida bloquean; Timestamp persiste como `{_seconds,_nanoseconds}` vía helpers existentes.
+- Array indices never offer Add (consistent with delete rule).
+
+## Verification evidence
+
+- TD-1 (commit 3623546): `pnpm typecheck` clean; `pnpm test` 12 files / 89 tests passed; scoped lint + prettier check on AddFieldDialog clean.
+- TD-2 (commit a22afac): `pnpm typecheck` clean; `pnpm test` 12 files / 89 tests passed; scoped lint + prettier check on CollectionTab, TreeView, TreeNodeRow, TreeContext, documentService clean.
+- Correction `faac5e2` (R3-002 parentPath fallback, R3-003 structuredClone): typecheck/test/scoped lint+format clean.
+- R3-001 fix `0207f8b` (timestamp invalid-date guard): typecheck/test/scoped lint+format clean.
+
+## Native review (RDD on, global) — ESCALATED, terminal
+
+- Assess on slice (`--base-ref 34923d6 --committed-only`): `risk: medium` (`executable_change` in CollectionTab), 566 changed lines.
+- Lineage `review-55ef78428f190c9d`, one lens `review-reliability`. Reviewer: 3 findings (R3-001 BLOCKER timestamp NaN, R3-002/R3-003 CRITICAL). Refuter corroborated.
+- Bounded correction `faac5e2` fixed R3-002 + R3-003 only. Validator rejected: R3-001 unaddressed → state `escalated`, then terminal `stop/native_stop_required`.
+- R3-001 fixed afterwards as ordinary work (`0207f8b`), outside the review transaction. Review outcome stays informational only — push/PR/merge remain user decisions under ordinary repository policy.
+
+## Next step
+
+- Delivery decision owned by the user: push / open PR / merge (slice is ~590 lines, over the 400-line budget — needs `size:exception` or split per repo policy).