Skip to content

Commit

Permalink
[repo] fix: missing sendActivity invoke for file consent accept/decli…
Browse files Browse the repository at this point in the history
…ne handlers (#2017)

## Linked issues

closes: #2004 

## Details

- for JS/PY, the handle wrappers were missing the sendActivity call,
resulting in 502s and automatic retries
- linting

## Attestation Checklist

- [x] My code follows the style guidelines of this project

- I have checked for/fixed spelling, linting, and other errors
- I have commented my code for clarity
- I have made corresponding changes to the documentation (updating the
doc strings in the code is sufficient)
- My changes generate no new warnings
- I have added tests that validates my changes, and provides sufficient
test coverage. I have tested with:
  - Local testing
  - E2E testing in Teams
- New and existing unit tests pass locally with my changes
  • Loading branch information
lilyydu authored Sep 13, 2024
1 parent d69ceb6 commit eb5a9fa
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 14 deletions.
16 changes: 12 additions & 4 deletions js/packages/teams-ai/src/Application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,8 +585,12 @@ export class Application<TState extends TurnState = TurnState> {
context.activity.value?.action === 'accept'
);
};
const handlerWrapper = (context: TurnContext, state: TState) => {
return handler(context, state, context.activity.value as FileConsentCardResponse);
const handlerWrapper = async (context: TurnContext, state: TState) => {
await handler(context, state, context.activity.value as FileConsentCardResponse);
await context.sendActivity({
type: ActivityTypes.InvokeResponse,
value: { status: 200 }
});
};
this.addRoute(selector, handlerWrapper, true);
return this;
Expand All @@ -607,8 +611,12 @@ export class Application<TState extends TurnState = TurnState> {
context.activity.value?.action === 'decline'
);
};
const handlerWrapper = (context: TurnContext, state: TState) => {
return handler(context, state, context.activity.value as FileConsentCardResponse);
const handlerWrapper = async (context: TurnContext, state: TState) => {
await handler(context, state, context.activity.value as FileConsentCardResponse);
await context.sendActivity({
type: ActivityTypes.InvokeResponse,
value: { status: 200 }
});
};
this.addRoute(selector, handlerWrapper, true);
return this;
Expand Down
6 changes: 1 addition & 5 deletions js/packages/teams-ai/src/StreamingResponse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,7 @@ describe('StreamingResponse', function () {
const activity = adapter.getNextReply();
assert.equal(activity.type, 'message', 'activity.type should be "message"');
assert.equal(activity.text, '', 'activity.text should be ""');
assert.deepEqual(
activity.channelData,
{ streamType: 'final' },
'activity.channelData should match'
);
assert.deepEqual(activity.channelData, { streamType: 'final' }, 'activity.channelData should match');
});
});

Expand Down
6 changes: 3 additions & 3 deletions js/packages/teams-ai/src/StreamingResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export class StreamingResponse {

/**
* Queues the next chunk of text to be sent to the client.
* @private
* @private
*/
private queueNextChunk(): void {
// Are we already waiting to send a chunk?
Expand Down Expand Up @@ -187,7 +187,7 @@ export class StreamingResponse {
// Send activity
await this.sendActivity(activity);
}

resolve();
} finally {
// Queue is empty, mark as idle
Expand Down Expand Up @@ -216,7 +216,7 @@ export class StreamingResponse {
if (!this._streamId) {
this._streamId = response?.id;
}
}
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion js/packages/teams-ai/src/models/OpenAIModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ export class OpenAIModel implements PromptCompletionModel {
if (!Array.isArray(params.tools) || params.tools.length == 0) {
if (params.tool_choice) {
delete params.tool_choice;
}
}
}

return params;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class ActionResponseValidator implements PromptResponseValidator<Validate
`No arguments were sent with called ${this._noun}. Call the "${function_call.name}" ${this._noun} with required arguments as a valid JSON object.`,
`The ${this._noun} arguments had errors. Apply these fixes and call "${function_call.name}" ${this._noun} again:`
);
const args = function_call.arguments === '{}' ? undefined : function_call.arguments ?? '{}';
const args = function_call.arguments === '{}' ? undefined : (function_call.arguments ?? '{}');
const message: Message = { role: 'assistant', content: args };
const result = await validator.validateResponse(
context,
Expand Down
12 changes: 12 additions & 0 deletions python/packages/ai/teams/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,12 @@ async def __handler__(context: TurnContext, state: StateT):
if not context.activity.value:
return False
await func(context, state, context.activity.value)
await context.send_activity(
Activity(
type=ActivityTypes.invoke_response,
value=InvokeResponse(status=200, body={}),
)
)
return True

self._routes.append(Route[StateT](__selector__, __handler__, True))
Expand Down Expand Up @@ -483,6 +489,12 @@ async def __handler__(context: TurnContext, state: StateT):
if not context.activity.value:
return False
await func(context, state, context.activity.value)
await context.send_activity(
Activity(
type=ActivityTypes.invoke_response,
value=InvokeResponse(status=200, body={}),
)
)
return True

self._routes.append(Route[StateT](__selector__, __handler__, True))
Expand Down

0 comments on commit eb5a9fa

Please sign in to comment.