Skip to content
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

Update command results to be the result cards of command executions #1947

Conversation

lukemelia
Copy link
Contributor

@lukemelia lukemelia commented Dec 17, 2024

  • Change how command results are stored in Matrix: old way: ReactionEvent + optional CommandResultEvent; new way: CommandResultEvent with optional data.content.cardEventId property that can point to a matrix-stored result card
  • Update aibot to read new result event
  • Update apply button state to read new result event
  • Replace special casing of search command result with rendering result card in embedded format in AI panel
  • Introduce CopyCard host command and refactor "copy to workspace" and other copy features to use it

…b) added as a card to the room, and c) references by eventID in the reaction event created once a command runs

aibot will need to be updated to read from this
Message model and room-message will need to updated to display the result card
Copy link

github-actions bot commented Dec 17, 2024

Host Test Results

    1 files  ±0      1 suites  ±0   20m 44s ⏱️ -34s
726 tests +3  724 ✔️ +3  2 💤 ±0  0 ±0 
731 runs  +3  729 ✔️ +3  2 💤 ±0  0 ±0 

Results for commit 8fa0992. ± Comparison against base commit fcbf10a.

♻️ This comment has been updated with latest results.

@lukemelia lukemelia force-pushed the cs-7598-ai-assistant-chat-should-automatically-display-return-value branch from 998f06d to ce4ffb2 Compare December 30, 2024 21:34
@lukemelia lukemelia force-pushed the cs-7598-ai-assistant-chat-should-automatically-display-return-value branch from ce4ffb2 to 4d942c1 Compare December 30, 2024 23:07
@lukemelia lukemelia changed the title WIP Updating command results to a) always be a CardDef or undefined, b) added as a card to the room, and c) references by eventID in the reaction event created once a command runs Update command results to be the result cards of command executions Jan 2, 2025
@lukemelia lukemelia force-pushed the cs-7598-ai-assistant-chat-should-automatically-display-return-value branch from 626b891 to 504856a Compare January 2, 2025 20:05
@lukemelia lukemelia marked this pull request as ready for review January 2, 2025 20:21
@lukemelia lukemelia requested review from IanCal and a team January 2, 2025 20:44
@@ -2,6 +2,7 @@ export const APP_BOXEL_CARDFRAGMENT_MSGTYPE = 'app.boxel.cardFragment';
export const APP_BOXEL_MESSAGE_MSGTYPE = 'app.boxel.message';
export const APP_BOXEL_COMMAND_MSGTYPE = 'app.boxel.command';
export const APP_BOXEL_CARD_FORMAT = 'app.boxel.card';
export const APP_BOXEL_COMMAND_RESULT_EVENT_TYPE = 'app.boxel.commandResult';
export const APP_BOXEL_COMMAND_RESULT_MSGTYPE = 'app.boxel.commandResult';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need a msgtype?

If we do, should we use this to distinguish between filled and undefined results? The event type says it's a command result, but the msgtype could be how to parse that (e.g. linked card or nothing). Notably we don't seem to use the msgtype on the bot side.

Copy link
Contributor Author

@lukemelia lukemelia Jan 7, 2025

Choose a reason for hiding this comment

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

Good idea. Updated this.

@IanCal
Copy link
Contributor

IanCal commented Jan 6, 2025

I like that responses with undefined don't trigger a response, that seems like a reasonable way to control some of this.

I failed to get this working with a command that returns a card defined in the same file. It's not known to the loader, and not in "localIdentities" so it fails:

image

Code https://gist.github.com/IanCal/78191aa82bfb51af2e88b81ca50b7234

@lukemelia
Copy link
Contributor Author

lukemelia commented Jan 6, 2025

I failed to get this working with a command that returns a card defined in the same file. It's not known to the loader, and not in "localIdentities" so it fails:

Hopefully will be fixed for cases where the command result card type is exported by this: #2004. We should still improve the error message for cases where there is a missing export.

@lukemelia lukemelia force-pushed the cs-7598-ai-assistant-chat-should-automatically-display-return-value branch from 4b7794a to ad7ee27 Compare January 7, 2025 15:39
IanCal and others added 2 commits January 7, 2025 10:39
- now used to identify results with a result card vs those without
@lukemelia lukemelia merged commit 1353a82 into main Jan 7, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants