FIX: apply diffs more consistently (#1367)

* FIX: apply diffs more consistently

1. Do escaping direct in diff streamer, that way HTML tags and other unsafe chars can be displayed and fixed
2. Add safeguard to ensure streaming always stops when it was terminated elsewhere

* lint

* bug unsubscribe should unsubscribe
This commit is contained in:
Sam 2025-05-24 15:19:48 +10:00 committed by GitHub
parent cead887480
commit 2d6ec5e1e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 29 additions and 12 deletions

View File

@ -24,8 +24,8 @@ export default class ModalDiffModal extends Component {
@tracked loading = false; @tracked loading = false;
@tracked finalResult = ""; @tracked finalResult = "";
@tracked selectedText = escapeExpression(this.args.model.selectedText); @tracked escapedSelectedText = escapeExpression(this.args.model.selectedText);
@tracked diffStreamer = new DiffStreamer(this.selectedText); @tracked diffStreamer = new DiffStreamer(this.args.model.selectedText);
@tracked suggestion = ""; @tracked suggestion = "";
@tracked @tracked
smoothStreamer = new SmoothStreamer( smoothStreamer = new SmoothStreamer(
@ -45,11 +45,16 @@ export default class ModalDiffModal extends Component {
// Prevents flash by showing the // Prevents flash by showing the
// original text when the diff is empty // original text when the diff is empty
return this.selectedText; return this.escapedSelectedText;
} }
get isStreaming() { get isStreaming() {
return this.diffStreamer.isStreaming || this.smoothStreamer.isStreaming; // diffStreamer stops "streaming" when it is finished with a chunk
return (
this.diffStreamer.isStreaming ||
!this.diffStreamer.isDone ||
this.smoothStreamer.isStreaming
);
} }
get primaryBtnLabel() { get primaryBtnLabel() {
@ -71,7 +76,7 @@ export default class ModalDiffModal extends Component {
@bind @bind
unsubscribe() { unsubscribe() {
const channel = "/discourse-ai/ai-helper/stream_composer_suggestion"; const channel = "/discourse-ai/ai-helper/stream_composer_suggestion";
this.messageBus.subscribe(channel, this.updateResult); this.messageBus.unsubscribe(channel, this.updateResult);
} }
@action @action
@ -105,7 +110,7 @@ export default class ModalDiffModal extends Component {
data: { data: {
location: "composer", location: "composer",
mode: this.args.model.mode, mode: this.args.model.mode,
text: this.selectedText, text: this.args.model.selectedText,
custom_prompt: this.args.model.customPromptValue, custom_prompt: this.args.model.customPromptValue,
force_default_locale: true, force_default_locale: true,
client_id: this.messageBus.clientId, client_id: this.messageBus.clientId,
@ -122,7 +127,7 @@ export default class ModalDiffModal extends Component {
if (this.suggestion) { if (this.suggestion) {
this.args.model.toolbarEvent.replaceText( this.args.model.toolbarEvent.replaceText(
this.selectedText, this.args.model.selectedText,
this.suggestion this.suggestion
); );
} }
@ -131,8 +136,12 @@ export default class ModalDiffModal extends Component {
this.finalResult?.length > 0 this.finalResult?.length > 0
? this.finalResult ? this.finalResult
: this.diffStreamer.suggestion; : this.diffStreamer.suggestion;
if (this.args.model.showResultAsDiff && finalResult) { if (this.args.model.showResultAsDiff && finalResult) {
this.args.model.toolbarEvent.replaceText(this.selectedText, finalResult); this.args.model.toolbarEvent.replaceText(
this.args.model.selectedText,
finalResult
);
} }
} }

View File

@ -2,8 +2,9 @@ import { tracked } from "@glimmer/tracking";
import { cancel, later } from "@ember/runloop"; import { cancel, later } from "@ember/runloop";
import loadJSDiff from "discourse/lib/load-js-diff"; import loadJSDiff from "discourse/lib/load-js-diff";
import { parseAsync } from "discourse/lib/text"; import { parseAsync } from "discourse/lib/text";
import { escapeExpression } from "discourse/lib/utilities";
const DEFAULT_CHAR_TYPING_DELAY = 30; const DEFAULT_CHAR_TYPING_DELAY = 10;
const STREAMING_DIFF_TRUNCATE_THRESHOLD = 0.1; const STREAMING_DIFF_TRUNCATE_THRESHOLD = 0.1;
const STREAMING_DIFF_TRUNCATE_BUFFER = 10; const STREAMING_DIFF_TRUNCATE_BUFFER = 10;
@ -51,7 +52,7 @@ export default class DiffStreamer {
const originalDiff = this.jsDiff.diffWordsWithSpace( const originalDiff = this.jsDiff.diffWordsWithSpace(
this.selectedText, this.selectedText,
this.suggestion newText
); );
this.diff = this.#formatDiffWithTags(originalDiff, false); this.diff = this.#formatDiffWithTags(originalDiff, false);
return; return;
@ -172,6 +173,10 @@ export default class DiffStreamer {
} }
async #streamNextChar() { async #streamNextChar() {
if (!this.isStreaming || this.isDone) {
return;
}
if (this.currentWordIndex < this.words.length) { if (this.currentWordIndex < this.words.length) {
const currentToken = this.words[this.currentWordIndex]; const currentToken = this.words[this.currentWordIndex];
@ -252,6 +257,7 @@ export default class DiffStreamer {
return `<span>${text}</span>`; return `<span>${text}</span>`;
} }
// returns an HTML safe diff (escaping all internals)
#formatDiffWithTags(diffArray, highlightLastWord = true) { #formatDiffWithTags(diffArray, highlightLastWord = true) {
const wordsWithType = []; const wordsWithType = [];
const output = []; const output = [];
@ -280,7 +286,8 @@ export default class DiffStreamer {
} }
for (let i = 0; i <= lastWordIndex; i++) { for (let i = 0; i <= lastWordIndex; i++) {
const { text, type } = wordsWithType[i]; let { text, type } = wordsWithType[i];
text = escapeExpression(text);
if (/^\s+$/.test(text)) { if (/^\s+$/.test(text)) {
output.push(text); output.push(text);
@ -310,6 +317,7 @@ export default class DiffStreamer {
i++; i++;
} }
chunkText = escapeExpression(chunkText);
output.push(this.#wrapChunk(chunkText, chunkType)); output.push(this.#wrapChunk(chunkText, chunkType));
} }
} }

View File

@ -647,7 +647,7 @@
.desktop-view & { .desktop-view & {
// a little extra space for extra narrow desktop view // a little extra space for extra narrow desktop view
@media screen and (width <= 675px) { @media screen and (max-width: 675px) {
span { span {
display: none; display: none;
} }