-
Notifications
You must be signed in to change notification settings - Fork 34
FEATURE: add inferred concepts system #1330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit adds a new inferred concepts system that: - Creates a model for storing concept labels that can be applied to topics - Provides AI personas for finding new concepts and matching existing ones - Adds jobs for generating concepts from popular topics - Includes a scheduled job that automatically processes engaging topics
* Adds support for concepts to be inferred from and applied to posts * Replaces daily task with one that handles both topics and posts * Adds database migration for posts_inferred_concepts join table * Updates PersonaContext to include inferred concepts
6c51329
to
74d47a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good. It'd be great to add some tests
], | ||
"deny": [] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this?
# frozen_string_literal: true | ||
class CreateInferredConceptsTable < ActiveRecord::Migration[7.2] | ||
def change | ||
create_table :inferred_concepts do |t| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using polymorphic association here instead of two more tables?
|
||
module DiscourseAi | ||
module InferredConcepts | ||
class Finder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Methods from this class that need a persona could be instance methods, and the persona could be passed in an initializer. DI makes it easier to test.
|
||
module DiscourseAi | ||
module InferredConcepts | ||
class Applier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Methods from this class that need a persona could be instance methods, and the persona could be passed in an initializer. DI makes it easier to test.
end | ||
|
||
def response_format | ||
[{ "key" => "matching_concepts", "type" => "array" }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add array
as an option in the persona's form
This commit adds a new inferred concepts system that: