[snyaggarwal]

Part of OpenConceptLab/ocl_online#465 (with OpenConceptLab/ocl_online#534).

What

Dialog layout (AutoMatchDialog.jsx)

• The top holds only the choices: target repository, "Input rows to match: N out of N total", and four radios with no sub-text. "Selected rows only" explains itself in a tooltip when disabled.
• "Retrieve candidates" and "Recommend matches with the OCL AI Assistant" (was "Run AI Analysis"), each with a short note.
• Notices move to a footer above Submit: replacing proposals (info), overwriting approved rows (warning, with the "I understand…" tick), AI-only rows, and the "nothing to run" blocks. A notice can ask for a tick or block Submit; changing the selection clears the ticks.

Runs change only the rows they include (MapProject.jsx, autoMatchRows.js)

• A run's rows are worked out at Submit, before anything changes: the scope's rows, at most getMaxRunRows of them, and for an AI-only run the rows the OCL AI Assistant can analyse.
• With no rows left, the run shows "nothing to run" and changes nothing.
• Only the rows the run includes lose their proposed or approved status (clearRunRowStatuses). Before, a run cleared statuses across its whole scope, including rows it never reached.

OCL AI Assistant default (getAutoMatchAIOnOpen)

• The first time the dialog opens after the project's logs load, the box follows the project's last run, and starts ticked when the project was never auto-matched. Until the logs load, the box is left alone.
• Later opens keep what the user left. aiOnByDefault() false opens it unticked every time.

Checkpoints (autoMatchCheck.js)

• useAutoMatchCheck(selection) returns {summary, notices}: a summary shown beside Submit, and notices that can carry confirm or blocks.
• aiOnByDefault(): whether the OCL AI Assistant may start ticked when the dialog opens.
• getMaxRunRows({matchAlgorithmCount}): the most rows a run may include, or null for all of them.
• The open stubs return nothing, true and null, so the open build runs every row in scope.
• Imported as @openconceptlab/oclmap-extensions. webpack.config.js resolves that name to the installed package when there is one, and to the stubs otherwise, and compiles that package's JSX with our .babelrc.
• getAutoMatchBlocker loses its AI-calls branch, and MapProject no longer works out its own row cut. The eligibleRowIndexes prop replaces previewEligibleRowIndexes on the dialog.

Build

• Production source maps are nosources-source-map: file names and line mappings, no source.
• Dockerfile: when the PRIVATE_PACKAGES_GIT build arg (an npm install spec) is set, installs it over SSH with --ignore-scripts, against GitHub's pinned host keys. It logs what it installed and fails if the package didn't install. Without the arg, nothing changes and the build needs no SSH.
• build.yml builds two images:
  • build: the open image, pushed to Docker Hub as before, with no private package or secrets.
  • build-private: the deploy image with the package, pushed only to ECR, with no Actions cache written. It uses the private-packages environment, SHA-pins webfactory/ssh-agent, and fails if the environment's secrets are missing. The deploy jobs now run this image.
• pr.yml is untouched, so PR CI builds the open stubs.

Strings

• 14 new keys in en, es, zh and pt-BR; the dialog's unused keys are removed. catalogKeys.test.js now accepts plural keys (_one/_other), the first in the catalogs. aiAssistantNaming.test.js covers the new strings that name the assistant.

Before merging

1. Merge OpenConceptLab/ocl_online#534.
2. Create the private-packages environment in this repo, with deployments allowed only from main, and put PRIVATE_GIT_PACKAGES_DEPLOY_KEY and PRIVATE_PACKAGES_GIT in it (values on OpenConceptLab/ocl_online#534). Until then build-private fails, and nothing releases or deploys.

Testing

• eslint clean; npm test 541 pass, npm run test:integration 6 pass. New unit tests cover takeRunRows, clearRunRowStatuses and getAutoMatchAIOnOpen.
• Docker, open (no build arg): builds; the install step doesn't run and needs no SSH; no checkpoint strings in the bundle; no map has sourcesContent.
• Docker, with the build arg: the package installs, the step logs its version and URL, and the bundle has its strings. The maps carry its file and identifier names but no source. With the wrong package, the build fails with a clear message.
• Rendered the dialog in a scratch harness: it matches the design, with the first run ticked and no summary.
• Not yet checked:
  • AutoMatch in the running app: the zero-row and shortened-run fixes and the AI default on slow logs are covered by unit tests only.
  • The build-private and deploy jobs on GitHub.

🤖 Generated with Claude Code

[snyaggarwal]

Verdict: Request changes

Issue complete: almost, but one accessibility regression should be fixed before merge.

[P2] Keep keyboard focus trapped inside the AutoMatch dialog

AutoMatchDialog.jsx now sets disableEnforceFocus on the MUI Dialog. That allows keyboard focus to move to controls behind the still-open modal, which breaks expected modal keyboard behavior and makes the dialog substantially harder to use with assistive technology. The new Tooltip/Alert content does not require opting out of the focus trap; please remove this prop unless there is a demonstrated nested-focus case and a replacement focus-management strategy.

Cross-repo check: the public/private checkpoint exports and selection contract match the companion package. The public diff keeps the newly moved commercial UI wording and calculations out of the open dialog.

Verification: CI is green; local oclmap test suite passes (535/535). Security blockers: none.

[snyaggarwal]

Fixed in 968a45b: removed disableEnforceFocus from the AutoMatch dialog, so focus stays trapped in the modal. Nothing in it needs the opt-out: the AI selector panel's Autocomplete keeps focus on its own input. Lint and tests pass (535/535).

[snyaggarwal]

Verdict: Approve

Re-review of 968a45b: the prior accessibility blocker is resolved. Removing disableEnforceFocus restores the Dialog's normal keyboard focus trap, and the fix introduces no other code changes.

Cross-repo review remains clean: the public checkpoint contract matches the private companion package and the open-core boundary is preserved. Verification: local oclmap tests pass (535/535), current CI and Snyk checks pass. Security blockers: none.

[snyaggarwal]

Verdict: Request changes

[P2] Keep the SSH setup conditional on the optional package

The updated Dockerfile now runs mkdir and ssh-keyscan github.com before checking PRIVATE_PACKAGES_GIT. That makes every ordinary build depend on outbound GitHub SSH access even when no optional package is configured; environments with port 22 blocked will fail before reaching the open stub. Move the SSH directory/keyscan setup back inside if [ -n "$PRIVATE_PACKAGES_GIT" ] so an unset build arg leaves the open build independent of the optional integration.

The npm-install-spec simplification otherwise looks correct. Current CI and Snyk checks pass. Security blockers: none.

[snyaggarwal]

Fixed in 776e6af: mkdir and ssh-keyscan are back inside if [ -n "$PRIVATE_PACKAGES_GIT" ], so a build without the arg makes no SSH call. Checked with a local build without the arg: the step finishes in 0.7s with no keyscan output, the image has no /root/.ssh and no optional package, and the webpack build completes. The step with the arg only moved lines inside the if and is otherwise unchanged from the build that installed the package earlier.

[snyaggarwal]

Verdict: Approve

Re-review of 776e6af: the prior Dockerfile blocker is resolved. SSH directory setup and ssh-keyscan now run only when PRIVATE_PACKAGES_GIT is non-empty, so an ordinary open build remains independent of GitHub SSH access while the configured install path is unchanged.

Verification: local oclmap tests pass (535/535); current CI and Snyk checks pass. The cross-repo checkpoint and open-core boundary remain clean. No correctness or security blockers found.

[paynejd]

Review of 776e6af

The dialog matches the approved design, and the public diff keeps to the open-core line. Two things should change before this merges:

• The open build can wipe proposals. A run the existing row cut empties now gets through and clears statuses.
• The build and deploy changes need more work before the secrets are set. That's in the private review on OpenConceptLab/ocl_online#534, along with everything on OCL Online's side.

Should fix before merging

1. A run cut to zero rows still clears proposals, and autosaves (P1). Codex F1.1 below, which I reproduced.

• MapProject.jsx still cuts a run's rows before running it (L2534-L2539; that code is tracked in OpenConceptLab/ocl_online#531).
• On main, the dialog disabled Submit when that cut would leave no rows. This PR moves that guard into the checkpoint, so the open build no longer has it.
• Meanwhile the run clears statuses for the whole scope before the cut (L2513-L2523).
• Reproduced with the real Mapper (open build) against a local mock API, with no production traffic. The case: a user whose cut leaves 0 rows, scope "Unmapped & proposed rows", 8 proposed rows.
  • Submit is enabled.
  • No run is created and no row is matched.
  • The autosave writes all 8 proposed rows back as Unmapped, with no message.
• Fix:
  • Work out the rows (after the cut) before touching statuses. If none are left, show auto_match_nothing_to_run and change nothing.
  • Clear statuses only for the rows the run will process, as the selected-rows branch already does. That also stops a shortened run from clearing proposals on rows it never reaches, which already happens on main.
  • Better still, the cut becomes a checkpoint answer, so the open build never cuts. Details on ocl_online#534.

2. Build and deploy (webpack.config.js, build.yml, Dockerfile). The findings and their fixes are on ocl_online#534. Two of them need fixing before the secrets are set.

Worth fixing

3. AutoMatch opened before the project logs load takes the first-run default (P2). Codex F1.2, reproduced.

• In the mock, the rows showed at 0.6 s and the logs took 8 s. Opening AutoMatch at 1.6 s ticked the OCL AI Assistant, although the last run had it off. Reopening after the logs loaded showed it unticked.
• A failed logs request does the same for the whole session.
• The default is also re-applied every time the dialog opens (L2963-L2964). Unticking it, closing the dialog to select rows and reopening ticks it again.
• Fix: record whether the logs loaded. Apply the default once, from loaded logs (or keep AutoMatch disabled until then), and leave the box alone on later opens.

4. Apply aiOnByDefault() on every open, not only on a project's first run. This is a design decision on OpenConceptLab/ocl_online#465, 2026-10-20.

• At L2964: setAutoRunAIAnalysis(aiOnByDefault() && getAutoMatchAIDefault(projectLogsRef.current, true)).
• In autoMatchCheck.js, the stub's comment changes to "Whether the OCL AI Assistant may start ticked when the dialog opens".
• The open stub returns true, so the open build doesn't change.

Notes (P3, no change needed unless you want it)

• Codex F1.3 (the AI model or prompt template doesn't clear ticks): no change needed. The ticks confirm row counts and overwrites, which the model doesn't change.
• PR CI (pr.yml) never runs webpack. A broken path to the stub would only show up in main's Docker build.
• The disabled "Selected rows only" tooltip keeps the old "Note:" prefix.

What I checked

• CI-exact checks:
  • Lint is clean, 535 unit and 6 integration tests pass.
  • Merged onto today's main (4fcd0a3), the merge is clean: lint is clean, 556 unit and 6 integration tests pass.
• Builds:
  • A production build without a package has none of the checkpoint's strings; with a package installed, they're there.
  • Docker builds with and without PRIVATE_PACKAGES_GIT. Without the arg, the SSH step doesn't run. With it, npm adds exactly one package.
• The dialog: rendered against the open stub across the design's scenarios. Notices, ticks clearing on selection change, the "Selected rows only" tooltip and the phone layout all match the mockup.
• The first-run default in the real Mapper: a project never auto-matched opens ticked, and a last run without the OCL AI Assistant opens unticked.
• The open-core rule: the added lines, comments, tests, locale files, commit messages and PR text name no plan, quota, cap, limit or allowance. getAutoMatchBlocker lost its AI-calls branch.
• Strings: the 14 new keys are in all four catalogs. zh correctly has only _other forms.

---

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

Verdict: findings (2 × P1, 1 × P2). Severity after my checks: F1.1 P1 (reproduced), F1.2 P2 (reproduced; it needs the dialog opened within the logs request), F1.3 P3 (see above).

• F1.1 P1 AutoMatchDialog.jsx:170: the open stub now permits empty runs that erase existing row statuses.
• F1.2 P1 MapProject.jsx:2964: unloaded project logs are treated as proof that this is the first AutoMatch.
• F1.3 P2 AutoMatchDialog.jsx:118: changing the AI model or prompt template does not clear confirmation ticks.

findings.json

{
  "reviewer": "codex-cli 0.163.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "776e6af7bc6c6dc76e1ab7475aa4a83d4c19b414",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "F1.1",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/AutoMatchDialog.jsx",
      "line": 170,
      "claim": "The open stub now permits empty runs that erase existing row statuses.",
      "scenario": "Without the private package, Submit remains enabled when the dialog has eligible rows but MapProject's subsequent truncation produces an empty rowsToProcess list. For 'Unmapped & proposed rows' or 'All rows, including approved', getRowsResults clears the corresponding proposed/approved statuses before applying that truncation, then schedules an autosave despite processing nothing. Those rows are consequently saved as unmapped. The removed Submit guard previously prevented this path.",
      "action": "Pass neutral execution eligibility to the dialog and reject empty runs before changing statuses. Restrict status changes to the rows actually processed, and add regression coverage.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.2",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/MapProject.jsx",
      "line": 2964,
      "claim": "Unloaded project logs are treated as proof that this is the first AutoMatch.",
      "scenario": "Reopen a project whose latest AutoMatch omitted with_ai_analysis. The input file can finish loading before the independent logs request, allowing AutoMatch to open while projectLogsRef.current is still empty. This enables AI using the first-run default. When the logs arrive, the checkbox is never corrected; a failed logs request leaves the same incorrect default. Later runs therefore do not reliably retain their last recorded AI choice.",
      "action": "Track whether project logs have loaded successfully and resolve the default from that authoritative result. Do not infer a first run from loading or failed history, and test delayed and failed log requests.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.3",
      "severity": "P2",
      "tag": null,
      "file": "src/components/map-projects/AutoMatchDialog.jsx",
      "line": 118,
      "claim": "Changing the AI model or prompt template does not clear confirmation ticks.",
      "scenario": "A staff user confirms overwriting approved rows, then selects a different AI model or prompt template in the dialog. Neither setting participates in confirmationKey, so the confirmation remains checked and Submit remains enabled for the changed selection, contrary to the acceptance criterion.",
      "action": "Include stable model and prompt-template identifiers in the confirmation reset dependencies. Add a lifecycle test that distinguishes selection changes from unrelated rerenders.",
      "lines_delta": null,
      "confidence": "high"
    }
  ]
}

[snyaggarwal]

Verdict: Approve

Re-review of a686d45: the prior blockers are resolved.

• The final run row set is determined before the run mutates state; an empty set is rejected, and only rows actually included in the run lose proposed/approved statuses.
• The AI default waits for loaded project history, applies once, and later dialog opens preserve the user's choice.
• The public Docker Hub image and the deploy image are now built separately. The open image has no optional extension, while the deploy build is isolated to its protected environment and private registry.
• The optional package loader uses the scoped package name, the open stub remains the fallback, and production source maps omit source content.

Cross-repo verification: the checkpoint exports match the companion package; the private package installs and compiles through the public webpack configuration.

Verification: local tests pass (541/541), the open and extension-backed production builds complete, and current PR CI/Snyk checks pass. No correctness, boundary, accessibility, or security blockers found.