mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix: address telegram review findings
This commit is contained in:
@@ -496,11 +496,12 @@ describe("dispatchTelegramMessage draft streaming", () => {
|
||||
|
||||
expect(answerDraftStream.forceNewMessage).not.toHaveBeenCalled();
|
||||
expect(editMessageTelegram).toHaveBeenCalledTimes(1);
|
||||
expect(editMessageTelegram.mock.calls[0]?.[0]).toBe(123);
|
||||
expect(editMessageTelegram.mock.calls[0]?.[1]).toBe(1001);
|
||||
expect(editMessageTelegram.mock.calls[0]?.[2]).toContain("Message A final");
|
||||
expect(editMessageTelegram.mock.calls[0]?.[2]).toContain("Message B final");
|
||||
expect(editMessageTelegram.mock.calls[0]?.[2]).toContain("Message C final");
|
||||
expect(editMessageTelegram).toHaveBeenCalledWith(
|
||||
123,
|
||||
1001,
|
||||
"Message A final Message B final Message C final",
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(deliverReplies).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -124,6 +124,17 @@ type DispatchTelegramMessageParams = {
|
||||
opts: Pick<TelegramBotOptions, "token">;
|
||||
};
|
||||
|
||||
type AnswerSegmentState = {
|
||||
text: string;
|
||||
finalized: boolean;
|
||||
implicitAfterFinal: boolean;
|
||||
};
|
||||
|
||||
type PendingAnswerFinalState = {
|
||||
payload: ReplyPayload;
|
||||
text: string;
|
||||
};
|
||||
|
||||
type TelegramReasoningLevel = "off" | "on" | "stream";
|
||||
|
||||
function resolveTelegramReasoningLevel(params: {
|
||||
@@ -253,8 +264,10 @@ export const dispatchTelegramMessage = async ({
|
||||
const answerLane = lanes.answer;
|
||||
const reasoningLane = lanes.reasoning;
|
||||
let splitReasoningOnNextStream = false;
|
||||
let answerSegmentPrefixText = "";
|
||||
let pendingAnswerFinalSlots = 1;
|
||||
const answerSegments: AnswerSegmentState[] = [];
|
||||
let answerBoundaryPending = false;
|
||||
const pendingAnswerFinals: PendingAnswerFinalState[] = [];
|
||||
const auxiliaryAnswerFinals: PendingAnswerFinalState[] = [];
|
||||
let bufferedAnswerFinal:
|
||||
| {
|
||||
payload: ReplyPayload;
|
||||
@@ -291,15 +304,91 @@ export const dispatchTelegramMessage = async ({
|
||||
Boolean(split.reasoningText) && suppressReasoning && !split.answerText,
|
||||
};
|
||||
};
|
||||
const getCurrentAnswerText = () => bufferedAnswerFinal?.text ?? answerLane.lastPartialText;
|
||||
const composeAnswerSegmentText = (text: string) =>
|
||||
appendAnswerSegment(answerSegmentPrefixText, text);
|
||||
const rememberAnswerBoundary = () => {
|
||||
answerSegmentPrefixText = getCurrentAnswerText();
|
||||
const composeAnswerSegmentsText = () =>
|
||||
answerSegments.reduce((acc, segment) => appendAnswerSegment(acc, segment.text), "");
|
||||
const getCurrentAnswerText = () => composeAnswerSegmentsText();
|
||||
const getLastAnswerSegment = () => answerSegments[answerSegments.length - 1];
|
||||
const getUnfinalizedAnswerSegments = () => answerSegments.filter((segment) => !segment.finalized);
|
||||
const bufferAnswerFinal = (payload: ReplyPayload) => {
|
||||
bufferedAnswerFinal = { payload, text: composeAnswerSegmentsText() };
|
||||
};
|
||||
const bufferAnswerFinal = (payload: ReplyPayload, text: string) => {
|
||||
bufferedAnswerFinal = { payload, text };
|
||||
answerSegmentPrefixText = text;
|
||||
const createAnswerSegment = (segmentStartsAfterFinal: boolean): AnswerSegmentState => {
|
||||
const segment: AnswerSegmentState = {
|
||||
text: "",
|
||||
finalized: false,
|
||||
implicitAfterFinal: segmentStartsAfterFinal && !answerBoundaryPending,
|
||||
};
|
||||
answerSegments.push(segment);
|
||||
answerBoundaryPending = false;
|
||||
return segment;
|
||||
};
|
||||
const commitAnswerFinal = (segment: AnswerSegmentState, final: PendingAnswerFinalState) => {
|
||||
segment.text = final.text;
|
||||
segment.finalized = true;
|
||||
segment.implicitAfterFinal = false;
|
||||
bufferAnswerFinal(final.payload);
|
||||
};
|
||||
const resolvePendingAnswerFinals = (opts?: { flushRemaining?: boolean }) => {
|
||||
if (pendingAnswerFinals.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
let unresolvedSegments = getUnfinalizedAnswerSegments();
|
||||
if (unresolvedSegments.length === 0) {
|
||||
const lastSegment = getLastAnswerSegment();
|
||||
if (!lastSegment || answerBoundaryPending) {
|
||||
unresolvedSegments = [createAnswerSegment(false)];
|
||||
} else {
|
||||
auxiliaryAnswerFinals.push(...pendingAnswerFinals.splice(0));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
!opts?.flushRemaining &&
|
||||
unresolvedSegments.length > 1 &&
|
||||
pendingAnswerFinals.length < unresolvedSegments.length
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const assignCount = Math.min(pendingAnswerFinals.length, unresolvedSegments.length);
|
||||
const segmentOffset =
|
||||
opts?.flushRemaining && pendingAnswerFinals.length < unresolvedSegments.length
|
||||
? unresolvedSegments.length - pendingAnswerFinals.length
|
||||
: 0;
|
||||
const assignedFinals = pendingAnswerFinals.splice(0, assignCount);
|
||||
const targetSegments = unresolvedSegments.slice(segmentOffset, segmentOffset + assignCount);
|
||||
|
||||
for (const [index, segment] of targetSegments.entries()) {
|
||||
const final = assignedFinals[index];
|
||||
if (!final) {
|
||||
continue;
|
||||
}
|
||||
commitAnswerFinal(segment, final);
|
||||
}
|
||||
|
||||
if (pendingAnswerFinals.length > 0) {
|
||||
auxiliaryAnswerFinals.push(...pendingAnswerFinals.splice(0));
|
||||
}
|
||||
};
|
||||
const updateAnswerSegmentFromPartial = (text: string) => {
|
||||
const lastSegment = getLastAnswerSegment();
|
||||
const segmentStartsAfterFinal = Boolean(lastSegment?.finalized);
|
||||
const needsNewSegment = answerBoundaryPending || !lastSegment || segmentStartsAfterFinal;
|
||||
const segment = needsNewSegment ? createAnswerSegment(segmentStartsAfterFinal) : lastSegment;
|
||||
if (text === segment.text) {
|
||||
return;
|
||||
}
|
||||
if (segment.text && segment.text.startsWith(text) && text.length < segment.text.length) {
|
||||
return;
|
||||
}
|
||||
segment.text = text;
|
||||
updateDraftFromPartial(answerLane, composeAnswerSegmentsText());
|
||||
};
|
||||
const queueAnswerFinal = (payload: ReplyPayload, text: string) => {
|
||||
pendingAnswerFinals.push({ payload, text });
|
||||
resolvePendingAnswerFinals();
|
||||
};
|
||||
const resetDraftLaneState = (lane: DraftLaneState) => {
|
||||
lane.lastPartialText = "";
|
||||
@@ -337,7 +426,7 @@ export const dispatchTelegramMessage = async ({
|
||||
updateDraftFromPartial(lanes.reasoning, segment.text);
|
||||
continue;
|
||||
}
|
||||
updateDraftFromPartial(lanes.answer, composeAnswerSegmentText(segment.text));
|
||||
updateAnswerSegmentFromPartial(segment.text);
|
||||
}
|
||||
};
|
||||
const flushDraftLane = async (lane: DraftLaneState) => {
|
||||
@@ -483,7 +572,11 @@ export const dispatchTelegramMessage = async ({
|
||||
},
|
||||
});
|
||||
const flushBufferedAnswerFinal = async () => {
|
||||
resolvePendingAnswerFinals({ flushRemaining: true });
|
||||
if (!bufferedAnswerFinal) {
|
||||
for (const auxiliaryFinal of auxiliaryAnswerFinals.splice(0)) {
|
||||
await sendPayload(auxiliaryFinal.payload);
|
||||
}
|
||||
return;
|
||||
}
|
||||
const { payload, text } = bufferedAnswerFinal;
|
||||
@@ -498,6 +591,9 @@ export const dispatchTelegramMessage = async ({
|
||||
infoKind: "final",
|
||||
previewButtons,
|
||||
});
|
||||
for (const auxiliaryFinal of auxiliaryAnswerFinals.splice(0)) {
|
||||
await sendPayload(auxiliaryFinal.payload);
|
||||
}
|
||||
reasoningStepState.resetForNextStep();
|
||||
};
|
||||
|
||||
@@ -556,19 +652,14 @@ export const dispatchTelegramMessage = async ({
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const answerText = composeAnswerSegmentText(segment.text);
|
||||
if (info.kind === "final") {
|
||||
if (pendingAnswerFinalSlots <= 0) {
|
||||
await sendPayload(payload);
|
||||
continue;
|
||||
}
|
||||
pendingAnswerFinalSlots -= 1;
|
||||
bufferAnswerFinal(payload, answerText);
|
||||
queueAnswerFinal(payload, segment.text);
|
||||
continue;
|
||||
}
|
||||
await deliverLaneText({
|
||||
laneName: "answer",
|
||||
text: answerText,
|
||||
text: composeAnswerSegmentsText(),
|
||||
sendText: segment.text,
|
||||
payload,
|
||||
infoKind: info.kind,
|
||||
previewButtons,
|
||||
@@ -636,10 +727,15 @@ export const dispatchTelegramMessage = async ({
|
||||
? () =>
|
||||
enqueueDraftLaneEvent(async () => {
|
||||
reasoningStepState.resetForNextStep();
|
||||
if (getCurrentAnswerText()) {
|
||||
pendingAnswerFinalSlots += 1;
|
||||
rememberAnswerBoundary();
|
||||
if (!getCurrentAnswerText()) {
|
||||
return;
|
||||
}
|
||||
const lastSegment = getLastAnswerSegment();
|
||||
if (lastSegment && !lastSegment.finalized && lastSegment.implicitAfterFinal) {
|
||||
lastSegment.implicitAfterFinal = false;
|
||||
return;
|
||||
}
|
||||
answerBoundaryPending = true;
|
||||
})
|
||||
: undefined,
|
||||
onReasoningEnd: reasoningLane.stream
|
||||
|
||||
@@ -4,6 +4,8 @@ import type { TelegramDraftStream } from "./draft-stream.js";
|
||||
|
||||
const MESSAGE_NOT_MODIFIED_RE =
|
||||
/400:\s*Bad Request:\s*message is not modified|MESSAGE_NOT_MODIFIED/i;
|
||||
const MESSAGE_NOT_FOUND_RE =
|
||||
/400:\s*Bad Request:\s*message to edit not found|MESSAGE_ID_INVALID|message can't be edited/i;
|
||||
|
||||
function isMessageNotModifiedError(err: unknown): boolean {
|
||||
const text =
|
||||
@@ -19,6 +21,20 @@ function isMessageNotModifiedError(err: unknown): boolean {
|
||||
return MESSAGE_NOT_MODIFIED_RE.test(text);
|
||||
}
|
||||
|
||||
function isMissingPreviewMessageError(err: unknown): boolean {
|
||||
const text =
|
||||
typeof err === "string"
|
||||
? err
|
||||
: err instanceof Error
|
||||
? err.message
|
||||
: typeof err === "object" && err && "description" in err
|
||||
? typeof err.description === "string"
|
||||
? err.description
|
||||
: ""
|
||||
: "";
|
||||
return MESSAGE_NOT_FOUND_RE.test(text);
|
||||
}
|
||||
|
||||
export type LaneName = "answer" | "reasoning";
|
||||
|
||||
export type DraftLaneState = {
|
||||
@@ -51,6 +67,7 @@ type CreateLaneTextDelivererParams = {
|
||||
type DeliverLaneTextParams = {
|
||||
laneName: LaneName;
|
||||
text: string;
|
||||
sendText?: string;
|
||||
payload: ReplyPayload;
|
||||
infoKind: string;
|
||||
previewButtons?: TelegramInlineButtons;
|
||||
@@ -196,6 +213,12 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) {
|
||||
return true;
|
||||
}
|
||||
if (args.treatEditFailureAsDelivered) {
|
||||
if (isMissingPreviewMessageError(err)) {
|
||||
params.log(
|
||||
`telegram: ${args.laneName} preview ${args.context} edit target missing; falling back to standard send (${String(err)})`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
if (args.context === "final") {
|
||||
args.lane.lastPartialText = args.text;
|
||||
}
|
||||
@@ -299,12 +322,14 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) {
|
||||
return async ({
|
||||
laneName,
|
||||
text,
|
||||
sendText,
|
||||
payload,
|
||||
infoKind,
|
||||
previewButtons,
|
||||
allowPreviewUpdateForNonFinal = false,
|
||||
}: DeliverLaneTextParams): Promise<LaneDeliveryResult> => {
|
||||
const lane = params.lanes[laneName];
|
||||
const deliveredPayloadText = sendText ?? text;
|
||||
const hasMedia = Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0;
|
||||
const canEditViaPreview =
|
||||
!hasMedia && text.length > 0 && text.length <= params.draftMaxChars && !payload.isError;
|
||||
@@ -342,9 +367,11 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) {
|
||||
);
|
||||
}
|
||||
await params.stopDraftLane(lane);
|
||||
const delivered = await params.sendPayload(params.applyTextToPayload(payload, text));
|
||||
const delivered = await params.sendPayload(
|
||||
params.applyTextToPayload(payload, deliveredPayloadText),
|
||||
);
|
||||
if (delivered) {
|
||||
lane.lastPartialText = text;
|
||||
lane.lastPartialText = deliveredPayloadText;
|
||||
}
|
||||
return delivered ? "sent" : "skipped";
|
||||
}
|
||||
@@ -361,7 +388,9 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) {
|
||||
params.log(
|
||||
`telegram: ${laneName} draft preview update not emitted; falling back to standard send`,
|
||||
);
|
||||
const delivered = await params.sendPayload(params.applyTextToPayload(payload, text));
|
||||
const delivered = await params.sendPayload(
|
||||
params.applyTextToPayload(payload, deliveredPayloadText),
|
||||
);
|
||||
return delivered ? "sent" : "skipped";
|
||||
}
|
||||
lane.lastPartialText = text;
|
||||
@@ -383,7 +412,9 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) {
|
||||
}
|
||||
}
|
||||
|
||||
const delivered = await params.sendPayload(params.applyTextToPayload(payload, text));
|
||||
const delivered = await params.sendPayload(
|
||||
params.applyTextToPayload(payload, deliveredPayloadText),
|
||||
);
|
||||
return delivered ? "sent" : "skipped";
|
||||
};
|
||||
}
|
||||
|
||||
@@ -177,6 +177,25 @@ describe("createLaneTextDeliverer", () => {
|
||||
expect(harness.log).toHaveBeenCalledWith(expect.stringContaining("keeping existing preview"));
|
||||
});
|
||||
|
||||
it("resends the final text when the preview message no longer exists", async () => {
|
||||
const harness = createHarness({ answerMessageId: 999 });
|
||||
harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found"));
|
||||
|
||||
const result = await harness.deliverLaneText({
|
||||
laneName: "answer",
|
||||
text: "Hello final",
|
||||
payload: { text: "Hello final" },
|
||||
infoKind: "final",
|
||||
});
|
||||
|
||||
expect(result).toBe("sent");
|
||||
expect(harness.editPreview).toHaveBeenCalledTimes(1);
|
||||
expect(harness.sendPayload).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ text: "Hello final" }),
|
||||
);
|
||||
expect(harness.log).toHaveBeenCalledWith(expect.stringContaining("edit target missing"));
|
||||
});
|
||||
|
||||
it("falls back to normal delivery when stop-created preview has no message id", async () => {
|
||||
const harness = createHarness();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user