[AI-3] Let administrators curate which models are offered by tangopium · Pull Request #24885 · opf/openproject

tangopium · 2026-08-22T14:55:10Z

Ticket

What are you trying to accomplish?

PR 7 of 11 in the AI-3 stack. Adds curation: a toggle per model row hides it from the default-model pickers or puts it back, stored as deactivated_at rather than in active, which the sync owns and rewrites on every refresh. Curation, not enforcement: anything already pointing at a hidden model keeps resolving. With a curated list to choose from, the connection form gains the default chat model selector, validated against the catalogue only on change so a shrinking catalogue does not block unrelated saves.

Merge checklist

Added/updated tests
Behind the llm_connection feature flag

Stacked on #24884.

github-actions · 2026-08-22T14:59:09Z

Caution

The provided work package version does not match the core version

Details:

Work package URL: https://community.openproject.org/work_packages/66020
Work package version: not set
Core version: 17.9.0

Please make sure that:

The work package version OR your pull request target branch is correct

github-actions · 2026-08-22T14:59:10Z

Caution

The Enterprise plan field is not set on the work package

Details:

Work package URL: https://community.openproject.org/work_packages/66020

Please make sure that:

The work package Enterprise plan field is set

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d70e87f0a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-08-22T15:50:57Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24885, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24885. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24885 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @tangopium to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @tangopium, and request a review from @tangopium.
On every commit, set @tangopium as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

Adds a toggle to each model row that hides it from the default-model pickers or puts it back, stored as deactivated_at rather than in active, which the catalogue sync owns and rewrites on every refresh. Curation, not enforcement: anything already pointing at a hidden model keeps resolving, so switching a row off can never silently break a running feature. With a curated list to choose from, the connection form gains the default chat model selector, backed by a contract validation that a designated default must be a model the server actually reported. It is validated only on change, so a catalogue that shrinks underneath a stored selection does not block every unrelated save. Part 7 of the AI-3 stack. https://community.openproject.org/work_packages/66020

github-actions · 2026-08-22T16:42:02Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]
rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24885, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24885. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24885 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @tangopium to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @tangopium, and request a review from @tangopium.
On every commit, set @tangopium as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

thykel

🏃‍♂️ Looks nice, getting there!

One additional observation: Currently, whenever I toggle the new button, I have to actually refresh the page to see the updated state within the table. This could feel a bit more lively.

However, the above point may be moot, since the only column that gets updated ("Source") is also the one that I propose to actually leave alone.

thykel · 2026-09-05T23:30:25Z

+
+require "spec_helper"
+
+RSpec.describe LlmModel, "deactivation", :llm_server_helpers, :webmock,


This is an oddly specific test suite, which does not fit our convention. Please establish a generic model spec which contains deactivation scenarios.

thykel · 2026-09-05T23:31:32Z

+    attribute :default_chat_model_id
+    attribute :default_embedding_model_id


Why are we repeating the lines from above? :)

thykel · 2026-09-05T23:32:30Z


    private

+    # The mirror image of default_embedding_model_can_embed: a model the server


default_embedding_model_can_embed does not exist yet. 👀

thykel · 2026-09-05T23:34:26Z

+    expect(connection.selectable_model_ids).to include("bge-m3")
+  end
+
+  # administrator switches off must never silently break a running feature.


🤖 #Testing ⚠️ Orphaned comment where the curation invariant should be tested

This comment has lost its first half, and it looks like it is marking a test that did not make it in.

The sentence it belongs to is the headline claim of the PR - "Curation, not enforcement: anything already pointing at a hidden model keeps resolving" - and it is asserted again in LlmModel#selectable?: "a feature already bound to a deactivated model keeps working... Switching a row off must never silently break a running feature."

The three examples here cover hiding from the pickers, surviving a sync, and being distinct from withdrawn. None covers that claim. The behaviour does exist in two places worth pinning down:

default_models_offered_by_server validates against available_model_ids, which includes deactivated models, so a stored default pointing at a hidden model still saves.

default_chat_model_options keeps the already-chosen id in the list, which exists precisely so a save cannot silently blank the field.

Could we add a case for each, and either finish or drop the comment? Whichever shape the file ends up in after the restructuring suggested above, these seem worth keeping.

thykel · 2026-09-05T23:34:27Z

+      # Withdrawn wins over hidden: the withdrawal is why the toggle is inert,
+      # and an administrator needs that explanation more than their own choice.
      return %i[attention source_withdrawn] if llm_model.withdrawn?
+      return %i[attention source_deactivated] if llm_model.deactivated?


🤖 #Design ⚠️ Source now answers two questions and drops provenance to do it

With this branch added, the ladder is withdrawn -> deactivated -> manual -> discovered, so a manually added model that an administrator switches off renders as "Hidden by administrator" and loses "Added manually". One column is answering both "where did this model come from" and "may it be used", and only the first of those has nowhere else to live.

This PR also adds the Status column right beside it, whose toggle already shows hidden versus shown - so the state is now rendered twice, and the duplicate is the one that costs us the provenance.

Would it work to leave Source as provenance only (Added manually / Reported by server / No longer reported) and let the toggle carry the administrator's choice on its own?

Minor related point: source_withdrawn and source_deactivated both render with :attention, so "the server stopped offering this" and "an administrator hid this" look identical while meaning quite different things.

The above is a bit long-winded, but we already hit upon the same point during the UX review -- the toggle shouldn't override the source field with completely different semantics.

This was referenced Aug 22, 2026

[AI-3] Probe models for embedding support automatically #24886

Open

[#66020] Add LLM connection settings to admin AI configuration #24851

Closed

chatgpt-codex-connector Bot reviewed Aug 22, 2026

View reviewed changes

Comment thread app/forms/llm_connections/connection_form.rb Outdated

Comment thread app/forms/llm_connections/connection_form.rb Outdated

Comment thread app/components/llm_connections/models_row_component.rb Outdated

tangopium force-pushed the feature/66020-pr07-model-curation branch from d70e87f to 710d953 Compare August 22, 2026 15:05

tangopium changed the title ~~[#66020] Let administrators curate which models are offered~~ [AI-3] Let administrators curate which models are offered Aug 22, 2026

tangopium force-pushed the feature/66020-pr07-model-curation branch from 710d953 to c87520f Compare August 22, 2026 15:26

tangopium force-pushed the feature/66020-pr07-model-curation branch from c87520f to b598b30 Compare August 22, 2026 15:51

tangopium force-pushed the feature/66020-pr07-model-curation branch from b598b30 to 3ccc2d0 Compare August 22, 2026 16:16

tangopium self-assigned this Aug 22, 2026

tangopium requested a review from thykel August 22, 2026 21:39

thykel reviewed Sep 5, 2026

View reviewed changes

Navigation Menu

Uh oh!

[AI-3] Let administrators curate which models are offered - #24885

[AI-3] Let administrators curate which models are offered#24885
tangopium wants to merge 1 commit into
feature/66020-pr06-manual-model-entriesfrom
feature/66020-pr07-model-curation

tangopium commented Aug 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

thykel left a comment

Uh oh!

thykel Sep 5, 2026

Uh oh!

thykel Sep 5, 2026

Uh oh!

thykel Sep 5, 2026

Uh oh!

thykel Sep 5, 2026

Uh oh!

thykel Sep 5, 2026

Uh oh!

thykel Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants


		require "spec_helper"

		RSpec.describe LlmModel, "deactivation", :llm_server_helpers, :webmock,

		attribute :default_chat_model_id
		attribute :default_embedding_model_id


		private

		# The mirror image of default_embedding_model_can_embed: a model the server

Uh oh!

Conversation

tangopium commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

Merge checklist

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

thykel left a comment

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

tangopium commented Aug 22, 2026 •

edited

Loading