Editing a letter drops producer/resumeId: partial save through saveLetter is a full replace

[s-annam]

LetterEditorDialog writes a partial record through saveLetter, but the store does a full replace. Every LetterRecord field the dialog does not name is silently deleted on save — including producer, which is the field the letter egress-acknowledgement gate reads.

Surfaced while reviewing #906. The behaviour is pre-existing on main, not introduced by that PR, so it wants its own issue.

The mechanism

putRecordVia (src/lib/storage/crud.ts:184-205) is the body behind putRecord:

const existing = (await db.get(store, record.id)) as T | undefined;
const written = {
  ...record,
  createdAt: existing?.createdAt ?? record.createdAt ?? now,
  updatedAt: options.touch === false
    ? (record.updatedAt ?? existing?.updatedAt ?? now)
    : now,
} as T;
await db.put(store, written);

existing contributes createdAt and updatedAt only. It is never spread. db.put replaces the whole value.

This is correct and deliberate — saveLetter's own docblock (letters.ts:47-67) states the resulting obligation on callers:

a stored letter may carry unknown extra keys the contract PRESERVED on import, and a housekeeping write that spreads a record back through here must not silently drop them

So the store's contract is "callers pass complete records". The bug is a caller that doesn't, plus a docblock asserting the store does the merging.

Where the merge is supposed to live

jobs gets this right, one layer up. updateJob (src/lib/job-tracker.ts:91) does the read-modify-write:

// job-tracker.ts:49
// `updateJob` spreads `{ ...existing, ...patch }`

resumes gets it right differently: saveResume (resumes.ts:30-38) names every field of ResumeRecord (id, filename, blob, parse), so there is nothing to drop. Fragile — adding a ResumeRecord field obliges updating saveResume — but correct today.

letters has neither. There is no updateLetter, and LetterEditorDialog calls the raw store wrapper with a partial:

// LetterEditorDialog.tsx — save()
await saveLetter({
  ...(letter?.id ? { id: letter.id } : {}),
  ...(jobId !== undefined ? { jobId } : {}),
  ...(companyKey !== undefined ? { companyKey } : {}),
  body,
  ...(label.trim() ? { label: label.trim() } : {}),
});

LetterRecord also carries resumeId and producer. Neither is sent, so both are destroyed on every edit.

What is actually lost

Field	Consequence
producer	The egress warning stops firing for that letter, permanently, on every surface. hasOutsideProducer in JobLetterIndicator reads letter.producer !== undefined; docs/cover-letter-contract.md §6 reads an absent producer as "written by offlinecv itself". One edit of an imported, producer-written letter relabels it as hand-typed and the disclosure is gone for good.
resumeId	The letter → saved-résumé link is dropped. clearLetterResumeLink exists as an explicit operation; this does it by accident.
label	Cannot be cleared. A blank label is sent as absent (...(label.trim() ? … : {})), so the old value survives. Read as a merge this looks intentional; read as a replace it is why emptying the field silently does nothing.
unknown extra keys	The ones saveLetter's docblock explicitly says must survive a round-trip.

The docblock is wrong, and it is load-bearing

LetterEditorDialog.tsx:15-23 justifies the egress rule with a claim about storage semantics:

A letter written HERE carries no producer block, and that absence is meaningful rather than incidental […] an existing record's own provenance survives the edit untouched (saveLetter spreads the input over the stored record, so keys it does not name are preserved).

The parenthetical is false. This matters more than a stale comment normally would: it is the stated reason the dialog is allowed to send a partial record, so the wrong comment is what makes the bug look safe to the next reader.

Secondary: no tombstone guard on letters

letters is one of the two stores that write deletedAt (types.ts:23-40), because it replicates. jobs guards resurrection explicitly — per getJob's docblock, updateJob "throws rather than quietly resurrecting it by writing a patch over the tombstone".

saveLetter has no such guard, and a full replace omitting deletedAt clears the tombstone. Not reachable from the UI today (getAllLetters filters tombstones, so the dialog never holds a deleted letter), but the letter store is a public contract for out-of-tree producers (#711), and a producer holding a stale id would resurrect a deleted letter with no error. Worth closing in the same change, since the fix is the same read-modify-write.

Proposed fix

Mirror the jobs shape rather than patching the call site:

1. Add updateLetter(id, patch) in a letters domain layer (the job-tracker.ts analogue), doing { ...existing, ...patch }, throwing on a missing-or-tombstoned id.
2. Point LetterEditorDialog.save() at it for the revise path; keep saveLetter for the insert path (composing, and the start-from copy, which must not carry an id).
3. Correct the LetterEditorDialog docblock to say what putRecordVia actually does — required whichever fix lands.
4. Decide label clearing deliberately: either send label: undefined explicitly so it can be emptied, or document that a label is not clearable.

A one-line ...(letter ?? {}) spread in the dialog fixes the field loss and nothing else. Acceptable as a stopgap, but it leaves the tombstone gap and repeats the pattern the next letter writer will also have to remember.

Acceptance criteria

• Editing an existing letter preserves producer, resumeId, and any unknown extra keys — asserted against a record carrying all three
• After editing a producer-written letter, the egress acknowledgement still fires for it (the feat(letters): job → company → standard resolution, picker, and customize-from #906 behaviour, tested end-to-end rather than by inspecting the record)
• A write against a tombstoned letter id is refused rather than resurrecting the record
• label clearing behaves as whichever way step 4 decides, with a test pinning it
• The LetterEditorDialog docblock no longer claims saveLetter merges
• The insert path still writes no id (the start-from copy stays a copy — Letter tiering UI: job → company → standard resolution, picker, and customize-from #767's whole model)
• npm run verify green

Out of scope

• saveResume's name-every-field style. Correct today; a separate call if anyone wants it made robust to new fields.
• saveJob / captureJob. Already covered by updateJob's merge at the domain layer.
• Changing putRecordVia to merge. It is deliberately a replace — importAll and the touch: false housekeeping writes depend on writing a record verbatim — and flipping it would silently change every store's write semantics.