Skip to content

Comments

feat: accept assistant.threads.setStatus method arguments from the assistant class#2660

Merged
zimeg merged 1 commit intoai-appsfrom
zimeg-feat-assistant-set-status-arguments
Sep 24, 2025
Merged

feat: accept assistant.threads.setStatus method arguments from the assistant class#2660
zimeg merged 1 commit intoai-appsfrom
zimeg-feat-assistant-set-status-arguments

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Sep 24, 2025

Summary

This PR accepts assistant.thread.setStatus method arguments from the Assistant class to support additional arguments.

Preview

const assistant = new Assistant({
  userMessage: async ({ setStatus }) => {
    await setStatus({
      status: "Thinking of something interesting...",
      loading_messages: [
        "Passing the time...",
        "Couting sheep...",
        "Thinking things...",
      ],
    });
  }
});

Requirements

@zimeg zimeg requested a review from mwbrooks September 24, 2025 04:01
@zimeg zimeg self-assigned this Sep 24, 2025
@zimeg zimeg added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented enhancement M-T: A feature request for new functionality semver:patch labels Sep 24, 2025
@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (ai-apps@8833205). Learn more about missing BASE report.

Additional details and impacted files 
@@            Coverage Diff             @@
##             ai-apps    #2660   +/-   ##
==========================================
  Coverage           ?   93.38%           
==========================================
  Files              ?       37           
  Lines              ?     7592           
  Branches           ?      668           
==========================================
  Hits               ?     7090           
  Misses             ?      497           
  Partials           ?        5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zimeg zimeg mentioned this pull request Sep 24, 2025
Merged
5 tasks
mwbrooks
mwbrooks approved these changes Sep 24, 2025
View reviewed changes
Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

✅ Looking good @zimeg! Thanks for updating this and retaining backwards compatibility!

test/unit/Assistant.spec.ts
Comment on lines +414 to +419
sinon.assert.calledWith(fakeClient.assistant.threads.setStatus, {
channel_id: 'C1234',
thread_ts: '1234.56',
status: 'Status set!',
});
});
Copy link
Member

@mwbrooks mwbrooks Sep 24, 2025

Choose a reason for hiding this comment

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

praise: This is a big improvement in test quality!

Copy link
Member Author

@zimeg zimeg Sep 24, 2025

Choose a reason for hiding this comment

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

🗣️ ramble: Thanks for noticing! I believe we should assert with as much detail as possible when possible to avoid unexpected regressions. Hope to continue with similar care to code health in related changes!

zimeg
zimeg commented Sep 24, 2025
View reviewed changes
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@mwbrooks Thanks so much for reviewing! I'm fascinated that these attributes can be overridden in calling code without much notice.

Let's merge this to continue improvements to the Assistant class 🤖

test/unit/Assistant.spec.ts
Comment on lines +414 to +419
sinon.assert.calledWith(fakeClient.assistant.threads.setStatus, {
channel_id: 'C1234',
thread_ts: '1234.56',
status: 'Status set!',
});
});
Copy link
Member Author

@zimeg zimeg Sep 24, 2025

Choose a reason for hiding this comment

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

🗣️ ramble: Thanks for noticing! I believe we should assert with as much detail as possible when possible to avoid unexpected regressions. Hope to continue with similar care to code health in related changes!

@zimeg zimeg merged commit 25f2071 into ai-apps Sep 24, 2025
19 checks passed
@zimeg zimeg deleted the zimeg-feat-assistant-set-status-arguments branch September 24, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mwbrooks mwbrooks mwbrooks approved these changes

Assignees

@zimeg zimeg

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented enhancement M-T: A feature request for new functionality semver:patch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zimeg @mwbrooks