Feature/sc 41194/ research this and cost it configure and by stevekaplan123 · Pull Request #3139 · Sefaria/Sefaria-Project

stevekaplan123 · 2026-03-09T13:25:11Z

Description

A brief description of the PR

Code Changes

The following changes were made to the files below

Notes

Any additional notes go here

…t-configure-and

mergify · 2026-03-09T14:12:49Z

🧪 CI Insights

Here's what we observed from your CI run for 9e25cf7.

🟢 All jobs passed!

But CI Insights is watching 👀

…41194/-research-this-and-cost-it-configure-and

Copilot

Pull request overview

This PR extends the ReaderApp chatbot integration by adding new configuration props (max prompts, welcome messages, interface language) and introducing a dedicated Django chatbot app to store editable welcome/restart text via the admin.

Changes:

Add a new Django chatbot app with ChatbotWelcomeMessage model, admin UI, migrations, and tests.
Extend base_props and ReaderApp.jsx to pass max-prompts, welcome-messages, and interface-lang into the <lc-chatbot> custom element.
Add a new remote-config key for feature.chatbot.max_prompts and enable the chatbot app in Django settings.

Reviewed changes

Copilot reviewed 12 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
static/js/ReaderApp.jsx	Passes additional chatbot attributes to the `<lc-chatbot>` element; relaxes display gating logic.
sefaria/settings.py	Adds the new `chatbot` Django app to `INSTALLED_APPS`.
remote_config/keys.py	Adds `CHATBOT_MAX_PROMPTS` remote-config key constant.
reader/views.py	Adds chatbot props (`chatbot_max_prompts`, `chatbot_welcome_messages`) to `base_props`.
reader/migrations/0002_add_chatbot_welcome_message.py	Introduces a temporary reader-owned welcome message model (later reverted).
reader/migrations/0003_revert_chatbot_welcome_message.py	Deletes the temporary reader-owned welcome message model.
chatbot/models.py	Implements `ChatbotWelcomeMessage` and cached `get_chatbot_welcome_messages()`.
chatbot/admin.py	Registers `ChatbotWelcomeMessage` with preview fields in Django admin.
chatbot/apps.py	Adds `ChatbotConfig` for the new app.
chatbot/migrations/0001_initial.py	Creates the `ChatbotWelcomeMessage` table and inserts a default record.
chatbot/migrations/0002_add_new_session_message.py	Adds `new_session_*` fields to the welcome message model.
chatbot/tests/test_chatbot_welcome_message.py	Adds tests for default and DB-backed welcome message behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T10:57:59Z

reader/views.py

    chatbot_data = {
        "chatbot_user_token": None,
        "chatbot_enabled": False,
        "chatbot_api_base_url": CHATBOT_API_BASE_URL,
        'chatbot_max_input_chars': remoteConfigCache.get(CHATBOT_MAX_INPUT_CHARS, default=500),
+        'chatbot_max_prompts': remoteConfigCache.get(CHATBOT_MAX_PROMPTS, default=100),
+        'chatbot_welcome_messages': json.dumps(get_chatbot_welcome_messages()),
    }


get_chatbot_welcome_messages() is called unconditionally when building base_props, which means every request pays at least a cache lookup (and a DB query on cache miss) even when the chatbot is disabled for the user. Consider only fetching/serializing welcome messages when _is_user_in_experiment(request) is true (or when chatbot_enabled will be true), since the <lc-chatbot> element is only rendered in that case anyway.

Copilot · 2026-03-10T10:58:00Z

sefaria/settings.py

 INSTALLED_APPS = (
    'adminsortable',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.messages',
    'reader',
+    'chatbot',
    'django.contrib.staticfiles',


INSTALLED_APPS currently adds the chatbot app as 'chatbot', but this repo already uses explicit AppConfig paths for some apps (e.g. remote_config.apps.RemoteConfigConfig, django_topics.apps.DjangoTopicsAppConfig). To keep behavior consistent on Django 1.11 and ensure any future ChatbotConfig.ready()/metadata is applied, add it as 'chatbot.apps.ChatbotConfig' (or add default_app_config in chatbot/__init__.py).

Copilot · 2026-03-10T10:58:00Z

chatbot/apps.py

+class ChatbotConfig(AppConfig):
+    default_auto_field = "django.db.models.BigAutoField"
+    name = "chatbot"


default_auto_field = "django.db.models.BigAutoField" is a Django 3.2+ setting and is unused on Django 1.11 (the version pinned in requirements.txt). Keeping it here can be misleading, especially since the migrations use AutoField; consider removing it (or documenting why it’s present) to avoid confusion.

Copilot · 2026-03-10T10:58:00Z

reader/migrations/0002_add_chatbot_welcome_message.py

+def create_default_welcome(apps, schema_editor):
+    ChatbotWelcomeMessage = apps.get_model("reader", "ChatbotWelcomeMessage")
+    ChatbotWelcomeMessage.objects.get_or_create(
+        key="default",
+        defaults={"welcome": "", "restart": ""},
+    )
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reader', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='ChatbotWelcomeMessage',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('key', models.CharField(default='default', help_text="Identifier for this message set (e.g. 'default')", max_length=50, unique=True)),
+                ('welcome', models.TextField(default='', help_text='Message shown when chat is empty (i.e. first-time)')),
+                ('restart', models.TextField(default='', help_text='Message shown on restart button')),
+                ('updated_at', models.DateTimeField(auto_now=True)),
+            ],
+            options={
+                'verbose_name': 'Chatbot welcome message',
+                'verbose_name_plural': 'Chatbot welcome messages',
+                'ordering': ['key'],
+            },
+        ),
+        migrations.RunPython(create_default_welcome, migrations.RunPython.noop),
+    ]


These two new reader migrations create ChatbotWelcomeMessage and then immediately delete it. Since the canonical model now lives in the new chatbot app, it would be cleaner to drop both reader migrations entirely (before merge) rather than shipping a create+drop sequence that adds noise to the migration history and performs unnecessary DDL on deploys.

Copilot · 2026-03-10T10:58:00Z

reader/migrations/0003_revert_chatbot_welcome_message.py

+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reader', '0002_add_chatbot_welcome_message'),
+    ]
+
+    operations = [
+        migrations.DeleteModel(name='ChatbotWelcomeMessage'),
+    ]


If reader/migrations/0002_add_chatbot_welcome_message.py is removed as suggested, this revert migration should also be removed. If there is a real need to keep a revert migration, consider renaming it to reflect the actual state transition (and add a data migration if existing installations have welcome message data that needs to be moved into the chatbot app).

Copilot · 2026-03-10T10:58:00Z

sefaria/settings.py

    'reader',
+    'chatbot',
    'django.contrib.staticfiles',


The PR title/description are still the template/placeholder text and don’t describe the intent of these chatbot changes (new app, welcome messages, remote config for max prompts). Please update the PR description to match the actual behavior change so reviewers can validate the requirements and rollout expectations.

stevekaplan123 added 17 commits February 23, 2026 15:16

chore: add CHATBOT_WELCOME_MESSAGES

49550a2

chore: first pass at Guide-like model for chatbot welcome messages

f8b4ef3

Merge branch 'master' into feature/sc-41912/welcome-experience-s

cd0eafd

Merge branch 'master' into feature/sc-41912/welcome-experience-s

b9ce173

Merge branch 'master' into feature/sc-41912/welcome-experience-s

abcdf8b

Merge branch 'master' into feature/sc-41912/welcome-experience-s

a91ef05

Merge branch 'master' into feature/sc-41912/welcome-experience-s

298567a

chore: hebrew + english first pass

bab0af4

Merge branch 'master' into feature/sc-41912/welcome-experience-s

42189fa

chore: bring back migration and revert it

005dd61

chore: add interface lang

1c5bca4

chore: add new session message

7742769

chore: max_prompts first pass

3aacbb3

Merge branch 'master' into feature/sc-41912/welcome-experience-s

0b4d78c

Merge branch 'master' into feature/sc-41194/-research-this-and-cost-i…

c79eefb

…t-configure-and

chore: add caching

80316de

chore: add make_preview

c5921f9

Merge branch 'feature/sc-41912/welcome-experience-s' into feature/sc-…

9e25cf7

…41194/-research-this-and-cost-it-configure-and

stevekaplan123 requested a review from Copilot March 10, 2026 10:53

Copilot started reviewing on behalf of stevekaplan123 March 10, 2026 10:53 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/sc 41194/ research this and cost it configure and#3139

Feature/sc 41194/ research this and cost it configure and#3139
stevekaplan123 wants to merge 18 commits intomasterfrom
feature/sc-41194/-research-this-and-cost-it-configure-and

stevekaplan123 commented Mar 9, 2026

Uh oh!

mergify bot commented Mar 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

stevekaplan123 commented Mar 9, 2026

Description

Code Changes

Notes

Uh oh!

mergify bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 CI Insights

🟢 All jobs passed!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mergify bot commented Mar 9, 2026 •

edited

Loading