Skip to content

OpenConceptLab/ocl_issues#2916 | A reopened project restores a mapped target's names and match highlight from its saved matches - #104

Merged
snyaggarwal merged 4 commits into
mainfrom
OpenConceptLab/ocl_issues#2916-restore-mapped-names
Oct 16, 2026
Merged

snyaggarwal merged 4 commits into
mainfrom
OpenConceptLab/ocl_issues#2916-restore-mapped-names

Conversation

@paynejd

@paynejd paynejd commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Stacked on oclmap#102 (based on its branch), so the diff below is this ticket's alone. GitHub retargets it to main when oclmap#102 merges. Merge the two together.

What changed and why

A reopened project rebuilt each mapped target from its CSV: only an id, a name and a URL. oclmap#102 fills in a target's names and match highlight from the row's restored candidates, but a target mapped from the Search tab isn't among them. So its grid cell led with the repository's name, not the input-language name, and its matched name never came back.

Every save already sends each mapped target's concept, names and search highlight included, in the project's matches, and opening a project returns it. Nothing read it back, so the first save after a reopen, autosave included, overwrote it with the stripped target.

  • Restoring (withSavedMatch, viewBuilders.js): setProjectFromData keys the saved matches by rowIndex once, and gives each target restored from the CSV the names and search_highlight its entry holds. An entry for another concept (another URL) is ignored. Only well-formed data comes back: names with a string name (and a string locale and type, if any), and highlights as lists of strings.
  • Keeping them per row (withCachedConcept): getConcept used to write each row's match into the cached concept that every row mapped to that concept shares. Rows mapped to the same concept could then show, and save, another row's match. It now returns a copy with the row's own names and match. That covers the grid cells and the save.
  • The row panel's Target Code header (withOwnNames) keeps the target's own names and highlight over those of a candidate for the same concept, which may have neither. So the header, the grid cell and the save agree.
  • Highlights render escaped (highlightHtml, conceptNames.js): only a highlight's <em> tags are markup, in the matched name and in Match Details. A highlight can now come from saved project data as well as from a search. The one-line matched name's tooltip uses the plain text.

No change to what's saved, and none to the API. A project whose matches entries lack names (saved before Feb 2026, or already overwritten by a save after a reopen) opens as it does today.

How I tested it

  • The steps of .github/workflows/pr.yml (Node 22.23.2): npm ci, eslint with --ext .jsx,.js (no problems), npm run test:all (506 unit tests and 6 integration tests pass).
  • New tests:
    • views.test.js, 6 tests. Two cover restoring: a target gets its saved names and highlight, and only well-formed ones. One checks that an entry for another concept, or none, leaves the target as the CSV restored it. One saves two rows mapped to one cached concept and reopens them, and each keeps its own match. Two more check that withCachedConcept copies rather than writes the cache, and that withOwnNames keeps a target's own over a candidate's. The two save tests fail if the cache is written instead of copied.
    • conceptNames.test.js, 1 test: only the highlight tags are markup, in the matched name and in highlightHtml. It fails without the escaping.
  • In a browser. I ran this branch's dev build against the staging API, anonymous and read-only. A synthetic saved project was served by Playwright routes: Spanish input language, and two targets mapped as from the Search tab. Row 1's target is also a semantic candidate with no names or highlight. Saves were answered locally, so nothing was written anywhere.
    • Grid: row 1 leads with "Hemoglobina en sangre", with "Matched name (it-IT): Emoglobina nel sangue" below it. The same holds after its row is opened.
    • Row panel header for row 1: the same name and matched name, with the candidate's 91% score.
    • Match Details for row 2, and its grid cell: a highlight's other markup shows as text.
    • Saving the reopened project sends both targets with their names (3 and 2) and highlights.
    • Control, with no entries (today's behavior): both rows lead with the repository's English name, with no matched name, and the save sends them with no names or highlight.
    • Entries for other concepts (swapped between rows): as the control.
    • Console: no page errors.

Acceptance criteria

  • A reopened project restores each mapped target's names and match highlight from its saved matches entry, when that entry is for the same concept.
  • Saving a reopened project keeps those names and that highlight in matches.
  • A project whose matches entries lack them opens as it does today.

Review

  • Codex pass 1 found 3 P1s (F1.1–F1.3), fixed in 777740f.
  • Pass 2 was clean.
  • A simplification review proposed 4 cuts, all applied in a146c7e.
  • An adversarial security pass found 2 P2s, fixed in fd87cdc, and a P3, recorded.
  • Codex's check of 777740f..fd87cdc was clean.

Not done

  • Entries a save already stripped can't be recovered. The fix holds from the next reopen on.

closes OpenConceptLab/ocl_issues#2916

🤖 Generated with Claude Code

paynejd and others added 2 commits October 15, 2026 21:57
…ach mapped target's names and match highlight from its saved matches

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… match per row, and escape highlights in Match Details

- getConcept returns a copy of the cached concept with the row's own names and
  match (withCachedConcept), instead of writing them into the shared entry
- the row panel keeps the target's own names and highlight over a candidate's
  (withOwnNames)
- Match Details renders highlights through highlightHtml, as the matched name does

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@paynejd paynejd left a comment •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

3 findings: 3 P1. (F1.1's wording is shortened here; the fix is in the response below.)

  • F1.1 · P1 · src/components/map-projects/viewBuilders.js:466 · Restored highlights reach Match Details, which renders them as HTML without escaping. (A saved matches entry holds a highlight with markup other than . Reopening the project and opening the mapped target's Match Details passes it through buildPanelPayload to MatchDetailsPanel, which renders it as HTML. getMatchedName's escaping covers only the inline matched name.)
  • F1.2 · P1 · src/components/map-projects/tests/views.test.js:779 · The round-trip test bypasses onSave, which still overwrites distinct highlights for rows sharing a cached target. (Reopen two rows mapped to the same concept with different saved highlights, then select a row so onCSVRowSelect caches that concept by URL. During onSave, getConcept returns and mutates the same cached object for both rows. Both serialized matches consequently contain the last row's highlight. Executing the actual getConcept and save-selection code reproduced this loss; the test directly serializes restored instead.)
  • F1.3 · P1 · src/components/map-projects/MapProject.jsx:1446 · Restored names and highlights are still overwritten when composing the row-panel target. (A Search-mapped target also appears among the row's algorithm candidates, such as a semantic candidate without highlights. Loading restores the saved highlight, but the targetConcept construction at lines 5350–5365 replaces its metadata with the candidate projection. The row panel loses the saved highlight; a candidate definition lacking names also replaces the restored localized name with the default name. Executing that construction reproduced both losses while the grid retained the restored values.)
findings.json
{
  "reviewer": "codex-cli 0.163.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "064516361cfe3121be049cb1e274b38ea0a11f5d",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "F1.1",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/viewBuilders.js",
      "line": 466,
      "claim": "Restored highlights reach Match Details, which renders them as HTML without escaping.",
      "scenario": "A saved matches entry holds a highlight with markup other than <em>. Reopening the project and opening the mapped target's Match Details passes it through buildPanelPayload to MatchDetailsPanel, which renders it as HTML. getMatchedName's escaping covers only the inline matched name.",
      "action": "Escape highlight text in MatchDetailsPanel as well, keeping only the emphasis tags, and test it.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.2",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/__tests__/views.test.js",
      "line": 779,
      "claim": "The round-trip test bypasses onSave, which still overwrites distinct highlights for rows sharing a cached target.",
      "scenario": "Reopen two rows mapped to the same concept with different saved highlights, then select a row so onCSVRowSelect caches that concept by URL. During onSave, getConcept returns and mutates the same cached object for both rows. Both serialized matches consequently contain the last row's highlight. Executing the actual getConcept and save-selection code reproduced this loss; the test directly serializes restored instead.",
      "action": "Build an independent concept snapshot per saved row without mutating the shared cache, and exercise the actual save path with two rows sharing a concept.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.3",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/MapProject.jsx",
      "line": 1446,
      "claim": "Restored names and highlights are still overwritten when composing the row-panel target.",
      "scenario": "A Search-mapped target also appears among the row's algorithm candidates, such as a semantic candidate without highlights. Loading restores the saved highlight, but the targetConcept construction at lines 5350–5365 replaces its metadata with the candidate projection. The row panel loses the saved highlight; a candidate definition lacking names also replaces the restored localized name with the default name. Executing that construction reproduced both losses while the grid retained the restored values.",
      "action": "Preserve the mapped target's restored names and highlight when merging candidate metadata into the row-panel target, and test an overlapping candidate with missing metadata.",
      "lines_delta": null,
      "confidence": "high"
    }
  ]
}

@paynejd

paynejd commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Response to Codex pass 1

F1.1 fixed in 777740f: Match Details renders highlights through the same highlightHtml as the matched name, so only <em> is markup in both places that render a highlight as HTML.
F1.2 fixed in 777740f: getConcept returns a copy of the cached concept with the row's own names and match (withCachedConcept), so rows sharing a concept no longer share one search_meta. That covers the save and the grid cells. The round-trip test now saves two rows sharing a cached concept through withCachedConcept.
F1.3 fixed in 777740f: the row panel works on a copy and puts the target's own names and highlight back over a candidate's (withOwnNames). The grid follows the same rule: with names among the row's own fields in withCachedConcept, a cell keeps its restored names after the row panel fetches the concept.

Checked in a browser too: a reopened target that overlaps a semantic candidate with no names or highlight keeps "Hemoglobina en sangre" and "Matched name (it-IT)" in both the row panel header and the grid, before and after the row is opened. Match Details shows a highlight's other markup as text.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review, pass 2 (codex-cli 0.163.0, commit 777740f)

Clean. F1.1, F1.2 and F1.3 resolved.

findings.json
{
  "reviewer": "codex-cli 0.163.0",
  "perspective": "defects",
  "mode": "verify",
  "commit": "777740f83b59d6fe6136b4827fcc55d9831e840f",
  "verdict": "clean",
  "prior": [
    {
      "id": "F1.1",
      "status": "resolved"
    },
    {
      "id": "F1.2",
      "status": "resolved"
    },
    {
      "id": "F1.3",
      "status": "resolved"
    }
  ],
  "findings": []
}

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplification review (commit 777740f)

4 items, about -5 lines.

  • S1 · yagni · src/components/map-projects/conceptNames.js:23 · highlightHtml's close parameter is never passed: both callers (getMatchedName and MatchDetailsPanel) rely on the '' default. → Make the signature (fragment, open = '<b>'), write .replaceAll('&lt;/em&gt;', '</b>'), and drop "and close" from the docstring. The escaping stays exactly as it is. (-1)
  • S2 · dup-test · src/components/map-projects/tests/views.test.js:769 · assert.equal(withCandidateNames(out, namedRowState, namedCache).names, searchedNames) repeats the base test 'withCandidateNames: keeps a target's own names...'. 718-7 has no candidate in namedRowState, so withCandidateNames returns the target unchanged, which assert.equal(withCandidateNames(searched, namedRowState, namedCache), searched) already pins. → nothing replaces it (-1)
  • S3 · delete · src/components/map-projects/MapProject.jsx:1419 · The two-line comment above savedMatches repeats withSavedMatch's JSDoc: names and highlight aren't in the CSV, and the ticket reference. → nothing replaces it; the withSavedMatch docstring on the next use already says this (-2)
  • S4 · shrink · src/components/map-projects/viewBuilders.js:487 · The docstring line "Returns cached as it is when there is none." restates the cached ? {...} : cached ternary. → Write export const withCachedConcept = (target, cached) => cached && {...cached, ...Object.fromEntries(...)} and drop that docstring line. (-1)
findings.json
{
  "reviewer": "claude subagent",
  "perspective": "simplify",
  "mode": "full",
  "commit": "777740f83b59d6fe6136b4827fcc55d9831e840f",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "S1",
      "severity": null,
      "tag": "yagni",
      "file": "src/components/map-projects/conceptNames.js",
      "line": 23,
      "claim": "highlightHtml's `close` parameter is never passed: both callers (getMatchedName and MatchDetailsPanel) rely on the '</b>' default.",
      "scenario": null,
      "action": "Make the signature `(fragment, open = '<b>')`, write `.replaceAll('&lt;/em&gt;', '</b>')`, and drop \"and `close`\" from the docstring. The escaping stays exactly as it is.",
      "lines_delta": -1,
      "confidence": "high"
    },
    {
      "id": "S2",
      "severity": null,
      "tag": "dup-test",
      "file": "src/components/map-projects/__tests__/views.test.js",
      "line": 769,
      "claim": "`assert.equal(withCandidateNames(out, namedRowState, namedCache).names, searchedNames)` repeats the base test 'withCandidateNames: keeps a target's own names...'. 718-7 has no candidate in namedRowState, so withCandidateNames returns the target unchanged, which `assert.equal(withCandidateNames(searched, namedRowState, namedCache), searched)` already pins.",
      "scenario": null,
      "action": "nothing replaces it",
      "lines_delta": -1,
      "confidence": "medium"
    },
    {
      "id": "S3",
      "severity": null,
      "tag": "delete",
      "file": "src/components/map-projects/MapProject.jsx",
      "line": 1419,
      "claim": "The two-line comment above `savedMatches` repeats withSavedMatch's JSDoc: names and highlight aren't in the CSV, and the ticket reference.",
      "scenario": null,
      "action": "nothing replaces it; the withSavedMatch docstring on the next use already says this",
      "lines_delta": -2,
      "confidence": "medium"
    },
    {
      "id": "S4",
      "severity": null,
      "tag": "shrink",
      "file": "src/components/map-projects/viewBuilders.js",
      "line": 487,
      "claim": "The docstring line \"Returns `cached` as it is when there is none.\" restates the `cached ? {...} : cached` ternary.",
      "scenario": null,
      "action": "Write `export const withCachedConcept = (target, cached) => cached && {...cached, ...Object.fromEntries(...)}` and drop that docstring line.",
      "lines_delta": -1,
      "confidence": "low"
    }
  ]
}

paynejd and others added 2 commits October 15, 2026 22:22
…ound unused or repeated

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… and highlights, and title the matched name with its plain text

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@paynejd

paynejd commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Response to the simplification review

S1 applied in a146c7e
S2 applied in a146c7e
S3 applied in a146c7e
S4 applied in a146c7e

Lint and npm run test:all pass after each commit.

@paynejd

paynejd commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Adversarial security pass (fresh Claude subagent, commit 777740f)

3 findings: 2 P2, 1 P3.

  • A1 · P2 · viewBuilders.js withSavedMatch · Saved names and highlights were restored without checking their types, so a malformed saved entry could stop the target from rendering. Fixed in fd87cdc: only names with a string name (and string locale and type, if any) and highlights as lists of strings are restored. New test with wrong-typed saved data.
  • A2 · P2 · Concept.jsx MatchedName · The one-line matched name's tooltip was built from the escaped HTML, so a name with an apostrophe or ampersand showed entity codes. Fixed in fd87cdc: getMatchedName also returns the plain text, and the tooltip uses it.
  • A3 · P3 · viewBuilders.js withCachedConcept / withOwnNames · A target's restored names take precedence over the names fetched from OCL when its row is opened. Recorded: they're the names from the project's target version, which the unversioned fetch may not return; they change only what's shown, not what's mapped or exported. An empty saved list no longer counts as names (A1's fix).

Also fixed in fd87cdc: highlightHtml's doc comment, garbled in a146c7e.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex check of commits 777740f..fd87cdc (codex-cli 0.163.0, commit fd87cdc)

Clean. It checked the simplification cuts (a146c7e) and the adversarial pass's fixes (fd87cdc).

findings.json
{
  "reviewer": "codex-cli 0.163.0",
  "perspective": "defects",
  "mode": "cuts",
  "commit": "fd87cdcc3b0edc9ce9f75a50b37d7aacb8b9fca2",
  "verdict": "clean",
  "prior": [],
  "findings": []
}

@paynejd
paynejd marked this pull request as ready for review October 16, 2026 02:35
@paynejd
paynejd requested a review from snyaggarwal October 16, 2026 03:07
Base automatically changed from OpenConceptLab/ocl_issues#2909-candidate-display-name to main October 16, 2026 03:46
@snyaggarwal
snyaggarwal merged commit 681ae62 into main Oct 16, 2026
2 checks passed
@snyaggarwal
snyaggarwal deleted the OpenConceptLab/ocl_issues#2916-restore-mapped-names branch October 16, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mapper: a reopened project forgets the names of a target mapped from the Search tab

2 participants