OpenConceptLab/ocl_online#81 | Using AI Assistant to generate change comment in concept#25
OpenConceptLab/ocl_online#81 | Using AI Assistant to generate change comment in concept#25snyaggarwal wants to merge 6 commits into
Conversation
paynejd
left a comment
paynejd
left a comment
There was a problem hiding this comment.
Review changes as discussed — most are inline suggestions you can commit directly. Two items can't be expressed as suggestions and are noted below.
1. New i18n keys (can't suggest — translations.json isn't in this PR's diff)
Add to src/i18n/locales/en/translations.json (and es/zh, or they'll fall back to en):
// "common": { ...
"generate_with_ai": "Generate with AI",
// "concept": { ...
"ai_assistant_not_configured": "AI assistant is not configured for this environment.",
"make_change_before_generating": "Make a change to the concept before generating a comment.",
"generate_comment_aria": "Generate comment with AI",
"try_again_in_a_moment": "Try again in a moment."2. Backend auth (context, not a code change here)
Server-side, POST /prompts/{key}/$invoke/ calls check_auth_group(MAPPER_AI_ASSISTANT_GROUP) (mapper_ai_assistant) — it is not superuser-gated. So the front-end visibility gate (superuser / core_user) and the back-end permission (mapper_ai_assistant) are different groups. That's the intended "safe even if it doesn't yet work for everyone" state — worth a follow-up ticket to align the groups before opening this to all users.
paynejd
left a comment
paynejd
left a comment
There was a problem hiding this comment.
Reviewed against ocl_online#81. Overall a clean, faithful implementation — env-var plumbing mirrors the existing ANALYTICS_API pattern, token forwarding works via currentUserToken(), metadata-exclusion lists match the spec, and the loading/disabled/tooltip states cover the acceptance criteria. A few inline notes below, plus one cross-repo dependency on backend permissions.
| const { conceptClasses, datatypes, locales, nameTypes, descriptionTypes, fields } = this.state | ||
| const { conceptClasses, datatypes, locales, nameTypes, descriptionTypes, fields, generatingChangeComment } = this.state | ||
| const aiAssistantConfigured = Boolean(this.getAIAssistantURL()) | ||
| const canSeeGenerateComment = edit && (isSuperuser() || hasAuthGroup(getCurrentUser(), 'core_user')) |
There was a problem hiding this comment.
This gates the button to core_users in addition to superusers, but per the ticket the AI Assistant $invoke endpoint still requires superuser — it notes this "must be relaxed to allow staff/authenticated users before this feature ships (separate ticket needed)." @snyaggarwal could you take the backend permissions work so core_users can actually invoke this? Otherwise non-superuser core_users will see the button and hit a 403.
There was a problem hiding this comment.
Good catch, raising separate PR
There was a problem hiding this comment.
Co-authored-by: Jonathan Payne <paynejd@gmail.com>
Co-authored-by: Jonathan Payne <paynejd@gmail.com>
2 participants
Linked Issue
Refs: OpenConceptLab/ocl_online#81