Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Conversation

@markvanlan
Copy link
Contributor

@markvanlan markvanlan commented Apr 23, 2025

This PR

  1. Abstracts the persona/llm dropdowns to a component to be used in both the composer connector AND the conversations page. (and adds them to the conversations page)
  2. Removes reliance on the composer for creating bot PMs on the new page. Now we simply make an ajax request and transition to the new post. This did require a new topic custom field, so that when we navigate, we don't have to rely on the "are any of the posts bot posts" logic -- that doesn't work when you transition immediately, as the bot hasn't posted yet. I don't think having this topic custom field is a bad thing.
  3. Simplify and improve how the update the topic titles in the sidebar on the left. No more updating DOM directly.

Screenshot 2025-04-23 at 10 50 09 AM

@markvanlan markvanlan changed the title Move persona/llm selector to component FEATURE: Allow for persona & llm selection in bot conversations page Apr 23, 2025
@markvanlan markvanlan marked this pull request as ready for review April 23, 2025 16:32
:topic_view,
:is_bot_pm,
include_condition: -> do
object.personal_message && object.topic.custom_fields[TOPIC_AI_BOT_PM_FIELD]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about existing PMs?

Copy link
Contributor Author

@markvanlan markvanlan Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xfalcox Doesn't mess with them.. they have an existing bot post. This would leave room in the future for us to somehow backfill.. but this is really to address the case of

  • PM is sent
  • Navigate to PM
  • There is no response yet.. we don't have any clue it's a bot PM (and the mechanisms to determine can have false-positive like the case with auto-responder)

I didn't remove the conditionals for checking if there is a bot PM, so old PMs still function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are adding a permanent custom field just to address the temporary case of detection during the first transition? I don't really agree littering the database permanently is a good strategy in this case.

Can't this be handled by a JS state, URL param, or anything else that has a more appropriate lifetime ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, we can --those all feel a bit hacky. I thought it would be actually useful to have a stable way to know what EXACTLY what PMs are bot conversations going forward.

I can do it another way, but I do want to ask one more time -- is there no value in being able to determine what PMs were started from this page?

@@ -0,0 +1,181 @@
import Component from "@glimmer/component";
Copy link
Contributor

@jjaffeux jjaffeux Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the big changes in this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing just copy/pasted into its own component?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply abstracting out the "setting" of values -- they're passed in now. instead of this.composer.metaData["personaId"] = id we have this.args.setPersonaId(id)

@markvanlan markvanlan merged commit b7b9179 into main Apr 24, 2025
6 checks passed
@markvanlan markvanlan deleted the llm-selector branch April 24, 2025 16:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants