OpenConceptLab/ocl_issues#2907 | suggesting field mapping for column headers

[snyaggarwal]

Linked Issue

Closes OpenConceptLab/ocl_issues#2907

[snyaggarwal]

Issue complete: no.

The suggestion rules and localization are covered and the current checks pass, but the new re-suggestion effect introduces a configuration-screen crash after a manual column edit when the available headers subsequently change. I left the reproduction and fix direction inline.

Verification: npm test (515 passed) and npm run eslint -- --quiet passed.

[snyaggarwal]

Issue complete: no.

The previous prev.some crash is fixed and now has good regression coverage. One remaining blocking data-correctness issue surfaced in the whole-PR pass: suggestions can duplicate a field that an exact header already maps, causing the exact input value to be overwritten when rows are prepared. I left the reproduction inline.

Verification: npm test (519 passed), npm run eslint -- --quiet passed, and CI/Snyk are green.

[snyaggarwal]

Verdict: approved / no findings.

Issue complete: yes. The previous runtime-shape regression and exact-mapping collision are both fixed. Exact and manually selected fields are now reserved before suggestions are assigned, regardless of column order, and upload/re-suggestion share the same covered path.

Security blockers: none.

Verification: npm test (525 passed), npm run eslint -- --quiet passed, and CI/Snyk are green.

GitHub does not permit formally approving one's own PR, so this is posted as a COMMENT review.

[paynejd]

Codex review, pass 1 (codex-cli 0.163.0, commit 14e174c)

Verdict: findings. One P1 confirmed as a regression; one P1 reads as P2; one P1 disagreed with; one P2 confirmed. Fixes are one-click suggestions in the review that follows.

• F1.1 (P1), confirmed. MapProject.jsx updateColumn: several source pickers on a multi-source Mapping: List column set their value in the same render. Simulated against the code: before this PR both target sources were kept (the old code mutated the column in place); at 14e174c only the last survives, so target_urls loses the others. Fix: a functional update that merges targetSource.
• F1.2 (P1 → P2), confirmed. Upload Notes,Test Name, then pick Name for Notes: Test Name keeps its suggested Name, and prepareRow lets the later column overwrite the manual one. Both columns show Name, so it's visible, but the manual choice should win. Fix: run resuggestColumns after a manual edit (same suggestion as F1.1).
• F1.3 (P1), disagree. An exact Description header is already a recognised field; the ticket's description → Name reads as a rule for headers like "Test description". Overriding an exact match would remap sheets that are already right. Open question for the ticket author: should a lone Description column, with no name-like column, be suggested as Name?
• F1.4 (P2), confirmed. Upload Specimen,Sample before the target loads, clear Specimen, then load LOINC: Specimen's discarded suggestion still holds Property: SYSTEM, so Sample gets nothing. Fix: suggest only for auto columns (one line, with a test that fails at 14e174c).

findings.json

{
  "reviewer": "codex-cli 0.163.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "14e174c7ce973d6dfb6dec2102f43e0a1709e11a",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "F1.1",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/MapProject.jsx",
      "line": 1323,
      "claim": "Batched automatic source selections overwrite one another.",
      "scenario": "Assign a column containing LOINC and SNOMED mappings to Mapping: List while both sources are available. Both source selectors run their mount effects against the same captured columns. The new immutable replacement retains only the last targetSource selection, dropping the other from matching configuration. The base commit preserves both.",
      "action": "Use functional state updates and merge each source selection into the latest targetSource state inside the updater. Add coverage for multiple selectors mounting together.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.2",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/MapProject.jsx",
      "line": 1323,
      "claim": "A manual mapping can remain overridden by an automatic suggestion.",
      "scenario": "Upload Notes,Test Name, then select Name for Notes. Test Name remains automatically mapped to Name because editing a column does not rerun suggestion reconciliation. prepareRow processes the later Test Name column last and silently overwrites the manually selected Notes value in the matching payload.",
      "action": "Reconcile remaining automatic suggestions whenever a manual mapping changes, giving manual selections precedence. Test the actual edit path without requiring a subsequent target reload.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.3",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/columnSuggestions.js",
      "line": 51,
      "claim": "A literal Description header never receives the required Name suggestion.",
      "scenario": "Upload Code,Description with concept names in Description. Since Description is already a valid field, the exact-match guard skips the description-to-Name rule. The column remains Description without a suggestion indicator, and prepareRow emits description rather than name, contrary to acceptance criterion 1.",
      "action": "Apply the required description-to-Name suggestion for new uploads before the exact-match exemption, while preserving saved user mappings. Test Description independently of other Name candidates.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.4",
      "severity": "P2",
      "tag": null,
      "file": "src/components/map-projects/columnSuggestions.js",
      "line": 89,
      "claim": "Manually edited columns consume suggestions that are then discarded.",
      "scenario": "Upload Specimen,Sample before target properties load, clear Specimen deliberately, then load a target supporting SYSTEM. suggestColumnLabels still processes Specimen's original header and reserves SYSTEM for it. That result is discarded because Specimen is no longer automatic, while Sample receives no suggestion despite the field being available.",
      "action": "Generate suggestions only for automatic columns, preserving their indexes, and reserve only the current mapped labels of manually edited columns.",
      "lines_delta": null,
      "confidence": "high"
    }
  ]
}

[paynejd]

Reviewed at 14e174c against OpenConceptLab/ocl_issues#2907. The rules, the neutral icon and the four locales look right. On the sample sheet (Code,Name,Specimen, LOINC target), Code gets the neutral icon, Name matches exactly and Specimen is suggested as Property: SYSTEM, so the red ✗ is gone.

Before merge

1. P1, "Learn more" returns 404. .../en/latest/oclmap/ has no index page, so AC 4 isn't met. Suggestion on constants.js.
2. P1, regression: a multi-source Mapping: List keeps only one target source. Codex F1.1, confirmed. Suggestion on updateColumn, which also fixes F1.2 (a manual choice should take its field back from a suggestion).

Smaller fixes
3. P2, Name goes to the first matching column, not the best one. "Test Date" or "Test Description" before "Test Name" takes Name.
4. P2, a cleared column still holds the field it would have been suggested. Codex F1.4.
5. P3, "External Name" is treated as a code column. Also P3: the "Suggested from column name" note comes back when a saved project is reopened.

With all the suggestions applied, CI's lint (eslint src --ext .jsx,.js) and npm run test:all pass: 528 unit and 6 integration tests. The two new tests for items 3 and 4 fail at 14e174c. Item 2 has no unit test, because the bug is in React state batching.

Timing: no need to hold this for the Oct 19 cut. Once these fixes are in, it's approved to go ahead.