Skip to content

OpenConceptLab/ocl_online#247 | repo version-create --match-algorithms, and a warning on version-update --match-algorithms - #12

Open
paynejd wants to merge 3 commits into
mainfrom
ocl_online-247-version-match-algorithms
Open

paynejd wants to merge 3 commits into
mainfrom
ocl_online-247-version-match-algorithms

Conversation

@paynejd

@paynejd paynejd commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Linked Issue

Part of OpenConceptLab/ocl_online#247 (the API side is OpenConceptLab/oclapi2#926).

Summary

  • ocl repo version-create --match-algorithms es,llm sends the new version's match algorithms in the create request, so a release can be created vectorized in one step. Without the flag the request leaves them out and the server decides. Once OpenConceptLab/ocl_online#247 | Release-level vectorization: vectors for every semantic repo version, reused instead of re-encoded oclapi2#926 is deployed, a new source version is vectorized when HEAD or the latest release is.
  • ocl repo version-update --match-algorithms now warns on stderr before sending the change. Changing an existing version's match algorithms makes the server reindex its concepts: adding llm embeds them, which can take hours for a large repository, and removing it can drop their vectors. The warning points to version-create --match-algorithms.
  • Sources only, never empty. --match-algorithms is refused for --type collection, which has no match algorithms, and refused when empty (an unset shell variable would otherwise clear them). Opting out is --match-algorithms es.
  • README: the vectorized-release example uses version-create, and a short note explains why.

The API already accepts match_algorithms in the version-create body, so the flag works against today's server too.

Test Plan

  • New tests/test_repo_commands.py (8 tests):
    • the flag reaches the client as a list, and is left out by default;
    • the client body includes match_algorithms only when it's given;
    • version-update warns with --match-algorithms, stays quiet without it, and keeps --json stdout parseable;
    • --match-algorithms is refused for collections, and so is an empty value.
  • pytest: 9 passed.
  • ruff check src tests: no new findings (the same 20 as main).
  • Codex adversarial review: 3 passes posted on this PR. Pass 3 is clean.

…s, and a warning on version-update --match-algorithms

- `repo version-create --match-algorithms es,llm` sets the new version's match algorithms in the create
  request. Without it the request leaves them out, so the server decides (a new source version is
  vectorized when HEAD or the latest release is).
- `repo version-update --match-algorithms` warns on stderr that changing an existing version's match
  algorithms makes the server reindex its concepts, and points to version-create instead.
- README: the vectorized-release example uses version-create.

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

@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 adversarial review, pass 1 (codex-cli 0.160.0, commit 8622165)

  1. Medium — src/ocl_cli/commands/repo.py:206–209; src/ocl_cli/api_client.py:929–930: source-only option is forwarded to collections.
    version-create O R v1 --type collection --match-algorithms es,llm sends match_algorithms to /orgs/O/collections/R/versions/, although collections have no such field. The update command likewise forwards it and prints an inapplicable reindexing warning. Depending on server validation, the request either fails or succeeds without enabling the requested matching behavior. Fix: reject this flag with --type collection before making a request, identify it as source-only in help/README, and test both commands. Consider guarding direct API-client calls too. The exact server response could not be verified offline.

  2. Low — tests/test_repo_commands.py:62–70: the warning test does not protect JSON stdout.
    It checks that stderr contains a warning but never parses stdout. An implementation that prints the warning to both streams would still pass while breaking ocl --json … | jq. Fix: assert that json.loads(result.stdout) equals the expected response, alongside the stderr assertions. The current implementation passes this stronger check in my independent reproduction.

What I checked and found correct:

  • Ran git diff origin/main...HEAD and inspected command dispatch, request construction, and output handling.
  • Whitespace is stripped and empty comma-separated items are discarded: " es, , llm, " becomes ["es", "llm"].
  • An explicit empty string or comma/whitespace-only value becomes [] and is sent. This is distinct from omission; callers using an empty shell variable will explicitly override server defaults. Clearing behavior is undocumented, but the supplied requirements do not establish that [] must be rejected.
  • Omitting the flag actually omits match_algorithms from the HTTP body. The client-level test correctly verifies this.
  • Update warnings go to stderr; stdout remains parseable JSON. Updates without the flag produce no warning.
  • Source creation uses the correct versions POST endpoint; update uses the version PATCH endpoint.
  • README’s default-vectorization and reindexing explanations match the supplied server behavior. The diff introduces no credentials or private tracker references into the public repository.
  • All 7 tests passed with bytecode writes disabled. Mocked HTTP reproductions confirmed the edge cases above. No files were modified and no network access was used.

No high-severity findings.

…lections; the warning keeps --json output clean

Codex review, pass 1:
- `repo version-create` and `repo version-update` refuse `--match-algorithms` with `--type collection`
  (a usage error, nothing sent): collections have no match algorithms (Medium). Help and README say
  "sources only".
- The version-update warning test now also checks stdout is the JSON response (Low).

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

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 1: dispositions (fixed in c808ed7)

  1. Medium: the source-only option is forwarded for collections. Fixed. version-create and version-update now refuse --match-algorithms with --type collection as a usage error (exit 2), and send nothing. Help and README say "sources only". Test: test_match_algorithms_are_refused_for_collections.
  2. Low: the warning test didn't protect JSON on stdout. Fixed. The test now parses stdout as the JSON response.

An empty --match-algorithms "" still sends [], so an explicit empty value stays distinct from leaving the flag out. That's deliberate.

@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 adversarial review, pass 2 (codex-cli 0.160.0, commit c808ed7)

  1. Medium — src/ocl_cli/commands/repo.py:185: Empty input silently becomes an explicit empty override.
    --match-algorithms "", whitespace-only input, and ",," all produce []. Both commands forward that list; the API client includes it in the request body. For example, a script using --match-algorithms "$ALGORITHMS" with an unset variable sends {"match_algorithms":[]}. On creation this overrides server inheritance; on update it requests clearing the algorithms, potentially removing vectorized matching. The current tests pass without covering this case. Fix: reject values with no nonempty algorithms using click.UsageError; add tests for empty, whitespace-only, and comma-only inputs on both commands. If clearing is intentional, document and test it explicitly.

What I checked and found correct:

  • Both pass-1 fixes hold: collections with the flag fail with exit code 2 before either client method runs, and the warning test parses JSON stdout.
  • Collections without the flag remain usable.
  • Whitespace is trimmed; empty items within "es,,llm," are discarded, yielding ["es", "llm"].
  • Omitting the flag results in no match_algorithms key in the create POST body.
  • Update warnings go to stderr; mocked JSON stdout remains parseable.
  • README wording matches the supplied server behavior and source-only restriction. No secrets or private infrastructure details appeared in the diff.
  • All 7 changed tests passed. Additional mocked probes confirmed the parsing edge cases above.

No files were modified and no network was used. Actual server handling of an empty list could not be verified offline.

…d instead of clearing them

Codex review, pass 2 (Medium): `--match-algorithms ""` (an unset shell variable, ",,", spaces) sent `[]`,
which overrides inheritance on create and opts a version out on update. Both commands now refuse it as a
usage error; opting out is `--match-algorithms es`.

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

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 2: dispositions (fixed in 2d457cb)

  1. Medium: an empty value silently becomes an explicit empty override. Fixed.
    • Both commands now refuse an empty, whitespace-only or comma-only --match-algorithms as a usage error (exit 2), and send nothing. The usual cause is an unset shell variable.
    • Opting a version out is --match-algorithms es.
    • Test: test_empty_match_algorithms_are_refused.

@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 adversarial review, pass 3 (codex-cli 0.160.0, commit 2d457cb): clean

No significant findings in git diff origin/main...HEAD. The pass 2 empty-value issue is fixed.

Checked and found correct:

  • Both commands reject empty, whitespace-only, and comma-only values before making an API call.
  • Parsing trims whitespace and drops empty items: " es , , llm, " becomes ["es", "llm"].
  • Omitting the create flag passes None, and the HTTP client omits match_algorithms from the POST body entirely.
  • Update warnings go to stderr; stdout remains valid JSON with --json. Omitting the flag produces no warning or update field.
  • Both commands reject --type collection --match-algorithms. Collections without the flag still work and omit the key.
  • POST/PATCH endpoints and payloads are correct in mocked request checks.
  • README matches the supplied server behavior and clearly states the source-only restriction. No secrets or private infrastructure details appear in the diff.

All 9 tests passed. Coverage could improve for mixed empty items and collection success without the flag; additional read-only checks confirmed both behaviors. Server behavior could not be independently verified offline.

No files modified; no network access used.

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.

1 participant