[AI-3] Register AI features and resolve their models at runtime - #24888
Open
tangopium wants to merge 10 commits into
Open
[AI-3] Register AI features and resolve their models at runtime#24888tangopium wants to merge 10 commits into
tangopium wants to merge 10 commits into
Conversation
This was referenced Aug 22, 2026
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
|
Caution The Enterprise plan field is not set on the work package Details:
Please make sure that:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd21ec891b
ℹ️ 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".
tangopium
force-pushed
the
feature/66020-pr10-features-and-runtime
branch
from
August 22, 2026 15:05
bd21ec8 to
de1e33c
Compare
tangopium
changed the title
tangopium
force-pushed
the
feature/66020-pr10-features-and-runtime
branch
from
August 22, 2026 15:26
de1e33c to
d435975
Compare
tangopium
force-pushed
the
feature/66020-pr10-features-and-runtime
branch
from
August 22, 2026 15:51
d435975 to
48829cb
Compare
Adds the feature registry: each AI feature declares its kind, the capabilities it requires and whether administrators may override its model, following the FeatureDecisions idiom and living in lib_static for the same reload-safety reason. The AI models page then lets an administrator bind each registered feature to a model, fed by a query that never hides a model: an option is selectable, selectable with a warning when a required capability is unknown, or disabled with the reason when it is ruled out. Llm::Runtime is the single place a feature's model is resolved: explicit override, then binding, then the connection default, and otherwise unbound. It fails closed with a machine-readable reason rather than substituting another model, because a text transform with a different model is a different feature. Bindings reference models by identifier string, never by foreign key, so a binding survives its model disappearing and the dangling state is derived, not stored. With something to bind, the connection form gains the default embedding model selector, offering only models actually known to embed, and the destructive dialogs now name the features that would be affected. Part 10 of the AI-3 stack. https://community.openproject.org/work_packages/66020
tangopium
force-pushed
the
feature/66020-pr10-features-and-runtime
branch
from
August 22, 2026 16:16
48829cb to
426df28
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy 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. |
The second default picker joins the chat one in the Default models section, so both defaults are chosen where model availability is controlled, as agreed in the UX review with Tom. It offers only models known to create embeddings, keeps a stored choice listed and flagged so a save cannot blank it, and points at the documentation about model types for administrators who do not know what an embedding model is. The contract now judges the embedding default by the model type, the same rule the picker offers by, so form, table and contract agree. The bindings page becomes Feature configuration and its description points at the AI models page where the defaults now live.
Following the UX review with Tom, the picker of a feature offers models of the feature's own kind instead of listing every stored model with a warning or a disabled entry. Semantic search sees only models known to create embeddings, and its caption says so and links to the documentation for administrators who do not know what an embedding model is. The model a feature is already bound to stays listed and choosable even once it no longer qualifies, flagged, so that opening the page and saving it cannot blank a working binding.
A feature that embeds now declares its document and query prefixes where it registers, defaulting to its own key, and the binding form starts out with them filled in. That was the request from the UX review with Tom: an administrator should not have to invent a prefix to configure semantic search. A prefix an administrator deliberately empties is still stored as empty, and a chat feature that passes a prefix is refused at registration time, since nothing would ever read it.
The AI section of the administration keeps a single entry, LLM settings. Feature configuration joins the connection and the AI models as its third tab, visible only while the connection is switched on, so the sidebar no longer lists three siblings that only make sense together. Admin::LlmFeatureBindingsController highlights the LLM settings entry and sends a disabled or missing connection back to the settings page, which makes the no-connection blankslate unreachable; it goes with its keys.
The contract judged the default embedding model by LlmModel#embedding?, which is false both for a model the server ruled out and for one nothing has probed yet. An unknown verdict is the normal state of a self-hosted server, so provisioning from the environment failed as soon as the catalogue carried a row for the configured model, while the identical configuration succeeded on an empty catalogue. Only a blocking verdict is refused now. The picker still offers confirmed models only, so nothing about the administrator's choice changes.
The embedding picker read capability off the list of selectable models, so switching off a model the server confirmed relabelled it as not known to create embeddings, and where it was the only one the caption asked the administrator to set a type that was already right. Whether a model embeds is a fact about the server; whether it is offered is the administrator's choice. LlmConnection#embedding_capable_model_ids now answers the first, and the form uses it for the label and the caption.
Opening the feature configuration URL while LLM features are switched off bounced the administrator back to the settings page in silence. It now carries the same notice the LLMs tab uses.
Nothing read them: Llm::Runtime::Resolution#embed passes only the model and the dimensions, while the caption promised that the document prefix is prepended to each document before it is indexed. An administrator was configuring fields that did nothing, and the prefix a model expects is a property of the embedder, not of an OpenProject feature. The embedding dimensions stay: the health check reads them and a stored index depends on them. The columns come off in a new migration rather than by editing 20260811140100_create_llm_feature_bindings.rb, whose commit is already on GitHub.
The feature configuration and the runtime still looked their default models up by name, which the connection now stores as references. The embedding picker offers rows, the runtime reads the identifier off the referenced row before asking the server for it, and both default validations work on the row rather than searching the catalogue for a string. The feature configuration and the runtime also ask the setting whether the AI features are on, rather than the column that used to answer it, and the connection factory gained transients so a spec can name the default it wants without knowing the row id.
tangopium
force-pushed
the
feature/66020-pr10-features-and-runtime
branch
from
September 10, 2026 06:52
426df28 to
5991d2b
Compare
Collaborator
Author
|
The branch was rebuilt, so the commits your comments point at no longer exist and those threads now render as outdated. Nothing was dropped. Here is where each point landed.
CI has not started on the new commits: only the CLA check ran, and the workflow runs seem to need maintainer approval on this repository. The checks are waiting on that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
AI-3
What are you trying to accomplish?
PR 10 of 11 in the AI-3 stack. Adds the feature registry (each AI feature declares its kind, required capabilities and whether its model may be overridden, following the FeatureDecisions idiom) and the AI models page that binds each registered feature to a model. Options are never hidden: a model is selectable, selectable with a warning when a required capability is unknown, or disabled with the reason when it is ruled out.
Llm::Runtimeis the single resolver (override, then binding, then connection default, otherwise unbound) and fails closed with a machine-readable reason rather than substituting another model. Bindings reference models by identifier string, never by foreign key, so they survive a model disappearing. This is the interface #77781 and #77783 consume.Merge checklist
llm_connectionfeature flagStacked on #24887.