Skip to content

DO NOT MERGE -- OpenConceptLab/ocl_issues#2922 | Add to Collection names the repo version being browsed in its expressions - #94

Draft
paynejd wants to merge 2 commits into
mainfrom
OpenConceptLab/ocl_issues#2922-add-references-repo-version
Draft

paynejd wants to merge 2 commits into
mainfrom
OpenConceptLab/ocl_issues#2922-add-references-repo-version

Conversation

@paynejd

@paynejd paynejd commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

What changed and why

TBv3 scopes concepts by the repo version in the URL, but the Add to Collection dialog built its expressions with dropVersion(c.url) and had no idea which repo version was selected. The API got no version, so it resolved the reference to the latest source version, whatever version the user was browsing. A concept retired in the latest version could end up in the collection as the retired concept.

Now, while browsing a version of a source, the expression names that version, in the API call preview and in the request:

/orgs/MyOrg/sources/MySource/concepts/A/ → /orgs/MyOrg/sources/MySource/v1/concepts/A/

  • toRepoVersionURI(uri, repo) in src/common/uri.js builds the expression. It uses the version only when the repo is a version of the resource's own source.
  • The dialog reads the source being browsed from contextRepo in OperationsContext, which covers all its callers: the concept list's bulk action, card view and the concept header. The ticket's starting point was to pass the version in from each caller, and the first commit did that. The simplification review replaced it: RepoHome already keeps this value and search's $match already reads its version_url, so six components don't need a new prop.
  • The API call preview now lists the expressions the dialog sends. With two or more concepts selected it used to show a /:owner/sources/:source/concepts/:id/ placeholder, which would have hidden the version.
Where the user is Expression
A source version, chosen or reached by opening the source Names that version
HEAD of a source Version-less, as now
Global search (no repo version selected) Version-less, as now
A collection or collection version Version-less, as now

How it was tested

PR checks (pr.yml, Node 22):

  • Lint and test passed on the last commit.
  • On an export of that commit, outside the repo as CI runs it: the secret-shaped variable check passed, npm ci passed, ./node_modules/eslint/bin/eslint.js src --ext .jsx,.js passed, npm test passed (119 tests).
  • In the session's worktree, the as-written ESLint command stops with a config error about a plugin loaded twice. That comes from the worktree being nested inside its repo; CI doesn't have that layout. With the repo's own config only (--no-eslintrc -c .eslintrc.json) it passes there too.

Unit tests: src/common/__tests__/uri.test.js adds 5 tests. With the base uri.js those 5 fail (114 pass, 5 fail); with the change, all 119 pass.

Browser check: headless Chromium with a fresh context at 1600×1000, and once at 1280×800. The app was served by npm start against the staging API, anonymously and read-only. The session was faked in the browser with no sign-in, and the script answered the PUT itself, so nothing reached staging. It drove the real dialog and compared the expressions in the API call preview with those in the captured PUT.

Before the change, the bulk action, concept header and card view on v2026-08-24 all sent the version-less /orgs/CIEL/sources/CIEL/concepts/170891/. On the final code, all 21 scenarios gave the expected expression (one was rerun alone, after fixing how the script picked its concept):

Scenario Expression in the preview and the PUT
v2026-08-24, not the latest: bulk action, one concept /orgs/CIEL/sources/CIEL/v2026-08-24/concepts/170891/
Same: bulk action, two concepts Both name v2026-08-24, in the preview too
Same: concept header /orgs/CIEL/sources/CIEL/v2026-08-24/concepts/170891/
Same: concept header at 1280×800 Same
Same: card view Same
Panel opened straight from its URL Same
Switching to v2026-07-20, then bulk action or concept header Names v2026-07-20
Back from v2026-07-20's panel to v2026-08-24's, with the panel still mounted Names v2026-08-24
Opening the source's bare URL, which lands on the latest release Names v2026-09-24
HEAD: bulk action, concept header, and switching to HEAD from a version Version-less
Global search: bulk action, two concepts from different sources, and concept header Version-less
A collection version: bulk action and concept header Version-less, /orgs/TO/sources/test-source/concepts/MA/
Moving inside the app from v2026-08-24 to a collection version, then adding a CIEL concept Version-less, /orgs/CIEL/sources/CIEL/concepts/168691/
Moving inside the app from v2026-08-24 to global search, then adding a CIEL concept Version-less, /orgs/CIEL/sources/CIEL/concepts/112141/
Moving inside the app from v2026-08-24 to global search, then the concept header Version-less
Moving inside the app from v2026-08-24 to another source's HEAD Version-less

I looked at the screenshots of the dialog, the preview and the results list. This change adds no strings, so no locale needed work.

Acceptance criteria

  • Adding a concept while browsing a specific repo version names that version in the preview and in the reference that gets added: the unit tests and the browser scenarios above.
  • Adding a mapping while browsing a specific repo version: nothing in TBv3 adds a mapping this way, so there's no flow to change. Neither the mappings tab's bulk actions nor a mapping's panel offers Add to Collection (checked in the browser on v2026-08-24). toRepoVersionURI handles mapping URIs and is tested on them.
  • On HEAD or with no version selected, the expression stays version-less: the unit tests and the HEAD and global search scenarios.

Docs

No doc describes how these expressions are built, so none changed. The repo has no docs/ folder, the README doesn't cover this flow, and changelog.md hasn't been edited since the first commit.

Review

  • Codex pass 1, on the first commit: clean.
  • Simplification review: 8 cuts, all applied, three with a change. S2 uses startsWith and slice instead of replace, S5 keeps the false assertion rather than undefined, and S6 keeps one assertion for another source.
  • Codex check of the commit with those cuts: clean.

No finding is open.

For the reviewer

The latest release now gets a versioned expression too. Opening a source lands on its latest release, and the URL names it, so that's indistinguishable from choosing the version. By the ticket's rule ("browsing a specific repo version") the expression names it. A reference added that way stays on that version when the source releases a newer one, where before it was a dynamic reference. If that default should stay dynamic, it's a small change, but it needs the ticket's rule to say so.

Chose not to do

  • Collections stay as they are. contextRepo isn't set on a collection's page, and a collection's version says nothing about the version of the concept's source. So adding from a collection still sends a version-less expression, which resolves to the latest source version: a CIEL concept shown in a collection at v2026-03-23 is sent as /orgs/CIEL/sources/CIEL/concepts/168691/. Whether that should name the concept's own source version instead is a question the ticket doesn't settle.
  • No Add to Collection action for mappings. That's a new feature, and the ticket names concept callers.

Noticed, not fixed

  • Clone to Source has the same gap. CloneToSourceDialog also builds its expressions with dropVersion(item.url), so it doesn't send the repo version being browsed either. I didn't test what the clone then resolves to. It's a separate flow from this ticket.
  • The concept's panel opens behind the card view's dialog. A click anywhere in a card's dialog, even on its title, opens the concept's panel and clears the card's checkbox. The dialog sits inside the card's onClick in the React tree. The screenshots show the same panel before and after this change.
  • Back between panels can show "this concept could not be found". Going back from v2026-07-20's panel to 170891's panel on v2026-08-24, where that concept exists, fetched /orgs/CIEL/sources/CIEL/v2026-07-20/concepts/170891/, got a 404 and showed the error. So the panel fetched with the version just left. This change doesn't touch that code; I didn't run this on the base branch.
  • Hard-coded English in the dialog. "Adding: … from …" is hard-coded although an addToCollection.adding_single key exists, and the results summary builds its text and plurals in English.

closes OpenConceptLab/ocl_issues#2922

🤖 Generated with Claude Code

https://claude.ai/code/session_01AaZ4qUrDxUHWvsX34Npdnf

Written by an Octoad session worker (worker-v0.17, recipe lean-loop, flow public>public, run openconceptlab-ocl-issues-2922-20261104-093906).

…sion being browsed in its expressions

The dialog built its expressions with dropVersion(c.url) and had no idea which repo version was selected, so the API
resolved them to the latest source version whatever version the user was browsing.

The concept list, card view and concept header now pass in the repo being browsed. toRepoVersionURI puts its version in
the expression when it is a version of the resource's own source. On HEAD, in global search and from a collection the
expression stays version-less, as it is now. The API call preview shows the expressions the dialog sends, also when
several concepts are selected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AaZ4qUrDxUHWvsX34Npdnf
@paynejd paynejd added origin/ai-generated Issue or PR created by an AI agent origin/session-worker Opened by the Octoad session worker; a run record backs it labels Nov 4, 2026

@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.166.0, commit b96b803)

Clean.

findings.json
{
  "reviewer": "codex-cli 0.166.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "b96b8036df981a0dba5e25e6852bfc0380614aa4",
  "verdict": "clean",
  "prior": [],
  "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 b96b803)

8 cuts.

S1 · reuse · src/components/common/AddToCollectionDialog.jsx:80 · The repo prop threaded through six components, since contextRepo in OperationsContext already holds the repo being browsed → the dialog reads contextRepo; the six other component files leave the diff (-5)
S2 · shrink · src/common/uri.js:28 · resourcePath and the concepts/mappings test, which only matter for a URI that already names a repo version → resourceURI.replace(dropVersion(repo.version_url), repo.version_url) (-6)
S3 · dup-test · src/common/tests/uri.test.js:33 · The user-owned source assertion, which takes the same path as the concept assertion → nothing replaces it (-4)
S4 · dup-test · src/common/tests/uri.test.js:41 · The concepts/v1/ and concepts/a%2Fb/ assertions, which take the same path → nothing replaces them; the test title loses "whatever the ids repeat" (-2)
S5 · dup-test · src/common/tests/uri.test.js:50 · The false and {} assertions, which take the same branch as undefined → nothing replaces them (-2)
S6 · dup-test · src/common/tests/uri.test.js:58 · The MySource2 and Other assertions, which fail the same check as the collection → nothing replaces them (-2)
S7 · dup-test · src/common/tests/uri.test.js:68 · The empty-string assertion, which takes the same branch as undefined → nothing replaces it (-1)
S8 · shrink · src/components/common/CascadeSelector.jsx:41 · previewExpressions, whose only job is the placeholder → a default parameter on expressions (-1)

findings.json
{"reviewer":"claude subagent","perspective":"simplify","mode":"full","commit":"b96b8036df981a0dba5e25e6852bfc0380614aa4","verdict":"findings","prior":[],"findings":[{"id":"S1","severity":null,"tag":"reuse","file":"src/components/common/AddToCollectionDialog.jsx","line":80,"claim":"Cut the `repo` prop threaded through six components: `contextRepo` in OperationsContext (src/components/app/LayoutContext.jsx) already holds the repo being browsed. RepoHome sets it to the same object as its `repo` state for a source (RepoHome.jsx:169), clears it on unmount (RepoHome.jsx:343) and remounts per repo (App.jsx:262), and Search.jsx:439 already reads it for `target_repo_url`. It is false in global search and in a collection, where the props end up version-less too, so the expressions come out the same. The cut covers Search.jsx:804, SearchResults.jsx:440 and :479, CardResults.jsx:6 and :28, ConceptCard.jsx:13 and :78, ConceptHome.jsx:384, and ConceptHeader's `browsedRepo` with its comment (ConceptHeader.jsx:24-26 and :175). The ticket's 'passed in from the caller' is its starting point, not its expected behavior.","scenario":null,"action":"In AddToCollectionDialog: `import { OperationsContext } from '../app/LayoutContext'`, `const { contextRepo } = React.useContext(OperationsContext)` and `toRepoVersionURI(c.url, contextRepo)`. The six other component files leave the diff.","lines_delta":-5,"confidence":"high"},{"id":"S2","severity":null,"tag":"shrink","file":"src/common/uri.js","line":28,"claim":"Cut `resourcePath` and the `/^(concepts|mappings)\\//` test. They only matter for a URI under the repo that isn't followed by concepts/ or mappings/, meaning one that already names a repo version, and `c.url` is never that: oclapi2's concept serializers return `versioned_object_url`, which is `drop_version(uri)`, with `uri` built from the HEAD source, and the ticket's step 3 shows the same. Every caller passes concepts only (SearchResults.jsx:283). Run side by side on 120 URI and repo pairs, the shorter form differs only on that case. Not checked against a live response. The test at uri.test.js:62-64 goes with it.","scenario":null,"action":"Lines 28-30 become `return resourceURI.replace(dropVersion(repo.version_url), repo.version_url)`; no match leaves the URI as it is. A `$` can't trip the replacement, since the API percent-encodes version ids in `version_url`.","lines_delta":-6,"confidence":"medium"},{"id":"S3","severity":null,"tag":"dup-test","file":"src/common/__tests__/uri.test.js","line":33,"claim":"Cut the third assertion, the user-owned source on version 2.0. It takes the same path as line 31 with different strings: the helper never reads the owner type or the version's format, and line 17 already covers /users/ in dropVersion.","scenario":null,"action":"nothing replaces it","lines_delta":-4,"confidence":"high"},{"id":"S4","severity":null,"tag":"dup-test","file":"src/common/__tests__/uri.test.js","line":41,"claim":"Cut the `concepts/v1/` and `concepts/a%2Fb/` assertions. Both take line 31's path: the helper never compares an id with the version and never decodes, and line 12 covers repeated ids in dropVersion.","scenario":null,"action":"nothing replaces it; the test title loses 'whatever the ids repeat'","lines_delta":-2,"confidence":"high"},{"id":"S5","severity":null,"tag":"dup-test","file":"src/common/__tests__/uri.test.js","line":50,"claim":"Cut the `false` and `{}` assertions: both take the same `!repo?.version_url` branch as `undefined` on line 49.","scenario":null,"action":"nothing replaces it","lines_delta":-2,"confidence":"high"},{"id":"S6","severity":null,"tag":"dup-test","file":"src/common/__tests__/uri.test.js","line":58,"claim":"Cut the `MySource2` and `Other` assertions. Both fail the same `startsWith` as COLLECTION_V1 on line 57, and `MySource2` doesn't pin the slash boundary, since `.../MySource2/` could never prefix `.../MySource/concepts/A/`.","scenario":null,"action":"nothing replaces it","lines_delta":-2,"confidence":"high"},{"id":"S7","severity":null,"tag":"dup-test","file":"src/common/__tests__/uri.test.js","line":68,"claim":"Cut the `''` assertion: it takes the same `!resourceURI` branch as `undefined` on line 67.","scenario":null,"action":"nothing replaces it","lines_delta":-1,"confidence":"high"},{"id":"S8","severity":null,"tag":"shrink","file":"src/components/common/CascadeSelector.jsx","line":41,"claim":"Cut `previewExpressions`: its only job is a placeholder when no expressions come in, and AddReferencesDialog, the one caller that passes none, gets that from a default parameter. An empty list would then preview as empty, which happens only when the dialog has nothing to send (AddToCollectionDialog.jsx:134).","scenario":null,"action":"`const CascadeSelector = ({ onChange, expressions = ['/:owner/sources/:source/concepts/:id/'], collectionUrl, showPreviewDefault = false }) => {` and `{ data: { expressions }, cascade: request.cascade }`.","lines_delta":-1,"confidence":"medium"}]}

…alog reads the source version from contextRepo, fewer tests

OperationsContext's contextRepo already holds the source being browsed, and search's $match already reads its
version_url, so the repo prop through six components goes. It is false in global search and from a collection, so
expressions stay version-less there as before.

toRepoVersionURI loses the guard for a URI that already names a repo version: the API's concept url never does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AaZ4qUrDxUHWvsX34Npdnf
@paynejd

paynejd commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Response to the simplification review

S1 applied in 352e10f. Only RepoHome sets contextRepo, for sources, and it remounts per repo and clears the value on unmount. The browser check ran again, with added moves between pages without a reload.
S2 applied in 352e10f, with startsWith and slice instead of replace, so nothing depends on how a $ in a version id is handled. A live response at a repo version confirmed the concept's url doesn't name that version, in the list and in the concept's own response.
S3 applied in 352e10f.
S4 applied in 352e10f.
S5 applied in 352e10f, cutting undefined and {} and keeping false, the value contextRepo holds.
S6 applied in 352e10f for the Other assertion. MySource2 kept: now that contextRepo is never a collection, another source is the case left for this check to catch.
S7 applied in 352e10f.
S8 applied in 352e10f.

@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 b96b803..352e10f

Clean.

findings.json
{
  "reviewer": "codex-cli 0.166.0",
  "perspective": "defects",
  "mode": "final",
  "commit": "352e10f4fef9e8efb287163232ce7a93438172ff",
  "verdict": "clean",
  "prior": [],
  "findings": []
}

@paynejd
paynejd marked this pull request as ready for review November 4, 2026 15:32
@paynejd paynejd changed the title OpenConceptLab/ocl_issues#2922 | Add to Collection names the repo version being browsed in its expressions DO NOT MERGE -- OpenConceptLab/ocl_issues#2922 | Add to Collection names the repo version being browsed in its expressions Nov 4, 2026
@paynejd
paynejd marked this pull request as draft November 4, 2026 16:19

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

origin/ai-generated Issue or PR created by an AI agent origin/session-worker Opened by the Octoad session worker; a run record backs it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TBv3: selected repo version is not sent in the add-references expression

1 participant