[snyaggarwal]

Closes OpenConceptLab/ocl_issues#2984

A review note is kept only in its row's matches entry, and a save sent entries only for mapped rows. Unmapped rows with a note now get a {rowIndex, note} entry, and reopening reads the note from it when the CSV row has none. A test covers the save and reopen round trip.

🤖 Generated with Claude Code

[snyaggarwal]

One review pass, P1–P5 considered.

Findings:

1. [P2] Explicitly unmapped rows can be emitted twice in matches: MapProject.jsx keeps rows that were manually unmapped, rejected, excluded, or bulk-cleared in mapSelected with a null value. The existing map(mapSelected, ...) therefore already creates a match entry for those rows. unmappedNoteMatches() tests only !mapSelected[index], so a noted row with a null entry is appended again, giving the save payload two entries with the same rowIndex. Please build one entry per row (or distinguish absent keys from present null keys) and add the mapSelected = {0: null} regression case.

2. [P3] The requested autosave/reopen path is not exercised: the new test calls two pure helpers and serializes their output, but never exercises the onSave wiring that appends to matches, nor the reopen wiring in setProjectFromData. That is why the duplicate-row case above passes unnoticed. Please cover the payload assembly and restore path closely enough to represent the issue’s edit → autosave → reopen acceptance criterion.

Checks: full test suite passes (651/651); focused ESLint passes. No P1, P4, or P5 findings. GitHub does not allow a request-changes verdict on one’s own PR, so this is posted as a review comment.

(snyaggarwal • Codex review)

[snyaggarwal]

Both review findings are fixed in the latest commit.

1. Duplicate entries: the save now builds its matches through one helper, buildMatchEntries. It makes one entry per row the project holds, a cleared row (null in mapSelected) included, and adds a note-only entry only for a noted row it holds no key for. A cleared row with a note is sent once and keeps its note, and a regression test covers mapSelected = {0: null}.
2. Round trip: the tests now go through the same payload assembly onSave uses, then serialise and restore with the reopen helper. The onSave and setProjectFromData calls themselves are not exercised by a component test, as the repo has no harness for the component; they are one-line calls to the tested helpers.

Full suite passes (652/652) and ESLint is clean on the changed files.

[snyaggarwal]

Re-review of e403fac (P1–P5 considered): no findings.

The previous P2 is resolved: buildMatchEntries now distinguishes an absent key from an explicitly cleared null row and emits one entry per row. The regression case covers the cleared-row path. The extracted helper is used directly by onSave, and the round-trip coverage exercises the same builder and restore helper, resolving the previous P3 test gap.

Checks: full suite passes (652/652); focused ESLint passes; diff check passes. GitHub does not permit approving one’s own PR, so this is recorded as a clean review comment.

(snyaggarwal • Codex re-review)