Add fingerprint to punctuation checker diagnostics

Author: ddaspit

.vscode/settings.json

@@ -7,5 +7,5 @@
       "pattern": "./packages/*/"
     }
   ],
-  "cSpell.words": ["clientrc", "sillsdev", "Usfm"]
+  "cSpell.words": ["checkables", "clientrc", "sillsdev", "Usfm"]
 }

package-lock.json

@@ -1,4 +1,18 @@
-import { concat, defer, from, map, merge, mergeMap, Observable, Subject, switchMap } from 'rxjs';
+import {
+  concat,
+  concatMap,
+  connectable,
+  defer,
+  finalize,
+  from,
+  map,
+  merge,
+  mergeMap,
+  Observable,
+  ReplaySubject,
+  Subject,
+  switchMap,
+} from 'rxjs';
 
 import { TextEdit } from '../common/text-edit';
 import { Document } from '../document/document';
@@ -23,26 +37,26 @@ export interface DiagnosticProvider<T = TextEdit> {
 
 export function activeDiagnosticsChanged$<T extends Document>(
   documents: DocumentAccessor<T>,
-  validateDocument: (doc: T) => Diagnostic[],
+  validateDocument: (doc: T) => Promise<Diagnostic[]>,
   refreshSubject?: Subject<string>,
 ): Observable<DiagnosticsChanged> {
   const streams: Observable<DiagnosticsChanged>[] = [
     documents.opened$.pipe(
-      map((e) => ({
+      concatMap(async (e) => ({
         uri: e.document.uri,
         version: e.document.version,
-        diagnostics: validateDocument(e.document),
+        diagnostics: await validateDocument(e.document),
       })),
     ),
     documents.changed$.pipe(
-      map((e) => ({
+      concatMap(async (e) => ({
         uri: e.document.uri,
         version: e.document.version,
-        diagnostics: validateDocument(e.document),
+        diagnostics: await validateDocument(e.document),
       })),
     ),
     documents.closed$.pipe(
-      switchMap(async (e) => {
+      concatMap(async (e) => {
         const doc = await documents.get(e.uri);
         return { uri: e.uri, version: doc?.version, diagnostics: [] };
       }),
@@ -51,9 +65,9 @@ export function activeDiagnosticsChanged$<T extends Document>(
   if (refreshSubject != null) {
     streams.push(
       refreshSubject.pipe(
-        switchMap(async (uri): Promise<DiagnosticsChanged> => {
+        concatMap(async (uri): Promise<DiagnosticsChanged> => {
           const doc = await documents.get(uri);
-          return { uri, version: doc?.version, diagnostics: doc != null ? validateDocument(doc) : [] };
+          return { uri, version: doc?.version, diagnostics: doc != null ? await validateDocument(doc) : [] };
         }),
       ),
     );
@@ -63,53 +77,76 @@ export function activeDiagnosticsChanged$<T extends Document>(
 
 export function allDiagnosticsChanged$<T extends Document>(
   documents: DocumentAccessor<T>,
-  validateDocument: (doc: T) => Diagnostic[],
+  validateDocument: (doc: T) => Promise<Diagnostic[]>,
   refreshSubject?: Subject<string>,
 ): Observable<DiagnosticsChanged> {
+  // Live document lifecycle streams that may emit while initial full validation is running.
   const streams: Observable<DiagnosticsChanged>[] = [
     documents.opened$.pipe(
-      map((e) => ({
+      concatMap(async (e) => ({
         uri: e.document.uri,
         version: e.document.version,
-        diagnostics: validateDocument(e.document),
+        diagnostics: await validateDocument(e.document),
       })),
     ),
     documents.changed$.pipe(
-      map((e) => ({
+      concatMap(async (e) => ({
         uri: e.document.uri,
         version: e.document.version,
-        diagnostics: validateDocument(e.document),
+        diagnostics: await validateDocument(e.document),
       })),
     ),
     documents.created$.pipe(
-      map((e) => ({
+      concatMap(async (e) => ({
         uri: e.document.uri,
         version: e.document.version,
-        diagnostics: validateDocument(e.document),
+        diagnostics: await validateDocument(e.document),
       })),
     ),
     documents.deleted$.pipe(map((e) => ({ uri: e.uri, diagnostics: [] }))),
     documents.reset$.pipe(
       switchMap((_) => documents.all()),
       mergeMap((docs) => docs),
-      map((doc) => ({ uri: doc.uri, version: doc.version, diagnostics: validateDocument(doc) })),
+      mergeMap(async (doc) => ({ uri: doc.uri, version: doc.version, diagnostics: await validateDocument(doc) })),
     ),
   ];
   if (refreshSubject != null) {
     streams.push(
       refreshSubject.pipe(
-        switchMap(async (uri): Promise<DiagnosticsChanged> => {
+        switchMap(async (uri) => {
           const doc = await documents.get(uri);
-          return { uri, version: doc?.version, diagnostics: doc != null ? validateDocument(doc) : [] };
+          return { uri, version: doc?.version, diagnostics: doc != null ? await validateDocument(doc) : [] };
         }),
       ),
     );
   }
-  return concat(
-    defer(() => from(documents.all())).pipe(
-      mergeMap((docs) => docs),
-      map((doc) => ({ uri: doc.uri, version: doc.version, diagnostics: validateDocument(doc) })),
-    ),
-    merge(...streams),
-  );
+
+  // Two-phase emission strategy:
+  // 1) Emit diagnostics for all currently known documents (initial snapshot pass).
+  // 2) Then continue with live document events.
+  //
+  // To avoid missing live events that occur during phase (1), we connect and buffer
+  // live streams immediately using a ReplaySubject-backed connectable observable.
+  return defer(() => {
+    const liveStreams$ = connectable(merge(...streams), {
+      connector: () => new ReplaySubject<DiagnosticsChanged>(),
+    });
+    // Start capturing live events now, before initial documents are validated.
+    const connection = liveStreams$.connect();
+
+    return concat(
+      // Initial full validation pass over all known documents.
+      defer(() => from(documents.all())).pipe(
+        mergeMap((docs) => docs),
+        mergeMap(async (doc) => ({ uri: doc.uri, version: doc.version, diagnostics: await validateDocument(doc) })),
+      ),
+      // Replay any buffered live events, then continue streaming new live events.
+      liveStreams$,
+    ).pipe(
+      finalize(() => {
+        // Ensure we tear down the live connection when downstream unsubscribes/completes.
+        connection.unsubscribe();
+      }),
+    );
+  });
 }

packages/core/src/diagnostic/diagnostic-provider.ts

@@ -31,13 +31,13 @@ export class VerseOrderDiagnosticProvider<T = TextEdit> implements DiagnosticPro
     if (validateAllDocuments) {
       this.diagnosticsChanged$ = allDiagnosticsChanged$(
         documents,
-        (doc) => this.validateDocument(doc),
+        (doc) => Promise.resolve(this.validateDocument(doc)),
         this.refreshSubject,
       );
     } else {
       this.diagnosticsChanged$ = activeDiagnosticsChanged$(
         documents,
-        (doc) => this.validateDocument(doc),
+        (doc) => Promise.resolve(this.validateDocument(doc)),
         this.refreshSubject,
       );
     }
@@ -112,7 +112,7 @@ export class VerseOrderDiagnosticProvider<T = TextEdit> implements DiagnosticPro
                 }),
                 moreInfo: this.localizer.t('verseOutOfOrder.moreInfo', { ns: 'verseOrder' }),
                 source: this.id,
-                fingerprint: `1|${chapterNumber}|${prevVerseNumber.toString()}`,
+                fingerprint: `1|${chapterNumber}:${prevVerseNumber.toString()}`,
               });
             }
           }
@@ -143,7 +143,7 @@ export class VerseOrderDiagnosticProvider<T = TextEdit> implements DiagnosticPro
           moreInfo: this.localizer.t('missingVerse.moreInfo', { ns: 'verseOrder' }),
           source: this.id,
           data: missingVerse,
-          fingerprint: `2|${chapterNumber}|${missingVerse.toString()}`,
+          fingerprint: `2|${chapterNumber}:${missingVerse.toString()}`,
         });
       }
     }

packages/examples/src/verse-order-diagnostic-provider.ts

@@ -34,6 +34,7 @@
     "directory": "packages/punctuation-checker"
   },
   "dependencies": {
+    "@aws-crypto/sha256-universal": "^5.2.0",
     "@sillsdev/lynx": "^0.3.5",
     "rxjs": "^7.8.1"
   },

packages/punctuation-checker/package.json

@@ -54,7 +54,7 @@ export abstract class AbstractChecker<TDoc extends TextDocument | ScriptureDocum
     if (doc == null) {
       return [];
     }
-    return this.validateDocument(doc);
+    return await this.validateDocument(doc);
   }
 
   async getDiagnosticFixes(uri: string, diagnostic: Diagnostic): Promise<DiagnosticFix<TEdit>[]> {
@@ -69,29 +69,31 @@ export abstract class AbstractChecker<TDoc extends TextDocument | ScriptureDocum
     this.refreshSubject.next(uri);
   }
 
-  private validateDocument(document: TDoc): Diagnostic[] {
+  private async validateDocument(document: TDoc): Promise<Diagnostic[]> {
     if (isScriptureDocument(document)) {
-      return this.validateScriptureDocument(document);
+      return await this.validateScriptureDocument(document);
     }
-    return this.validateTextDocument(document);
+    return await this.validateTextDocument(document);
   }
 
-  protected validateTextDocument(textDocument: TextDocument): Diagnostic[] {
+  protected async validateTextDocument(textDocument: TextDocument): Promise<Diagnostic[]> {
     const diagnosticFactory: DiagnosticFactory = new DiagnosticFactory(this.id, textDocument);
 
     const issueFinder: IssueFinder = this.issueFinderFactory.createIssueFinder(diagnosticFactory);
-    return issueFinder.produceDiagnostics(new CheckableGroup([new TextDocumentCheckable(textDocument.getText())]));
+    return await issueFinder.produceDiagnostics(
+      new CheckableGroup([new TextDocumentCheckable(textDocument.getText())]),
+    );
   }
 
-  protected validateScriptureDocument(scriptureDocument: ScriptureDocument): Diagnostic[] {
+  protected async validateScriptureDocument(scriptureDocument: ScriptureDocument): Promise<Diagnostic[]> {
     let diagnostics: Diagnostic[] = [];
     const diagnosticFactory: DiagnosticFactory = new DiagnosticFactory(this.id, scriptureDocument);
 
     const issueFinder: IssueFinder = this.issueFinderFactory.createIssueFinder(diagnosticFactory);
 
     const scriptureNodeGrouper: ScriptureTextNodeGrouper = new ScriptureTextNodeGrouper(scriptureDocument);
     for (const checkableGroup of scriptureNodeGrouper.getCheckableGroups()) {
-      diagnostics = diagnostics.concat(issueFinder.produceDiagnostics(checkableGroup));
+      diagnostics = diagnostics.concat(await issueFinder.produceDiagnostics(checkableGroup));
     }
     return diagnostics;
   }

packages/punctuation-checker/src/abstract-checker.ts

@@ -31,40 +31,63 @@ export class AllowedCharacterIssueFinder implements IssueFinder {
     this.diagnosticList = new DiagnosticList();
   }
 
-  public produceDiagnostics(checkableGroup: CheckableGroup): Diagnostic[] {
+  public async produceDiagnostics(checkableGroup: CheckableGroup): Promise<Diagnostic[]> {
     this.diagnosticList = new DiagnosticList();
 
     for (const checkable of checkableGroup) {
       let match: RegExpExecArray | null;
-      while ((match = this.characterRegex.exec(checkable.getText()))) {
+      const text = checkable.getText();
+      while ((match = this.characterRegex.exec(text))) {
         const character = match[0];
 
-        this.checkCharacter(character, match.index, match.index + match[0].length, checkable.getEnclosingRange());
+        await this.checkCharacter(
+          character,
+          match.index,
+          match.index + match[0].length,
+          checkable.getEnclosingRange(),
+          checkable.getVerseRef(),
+          checkable.getLeftContext(match.index, 5),
+          checkable.getRightContext(match.index + match[0].length, 5),
+        );
       }
     }
 
     return this.diagnosticList.toArray();
   }
 
-  private checkCharacter(
+  private async checkCharacter(
     character: string,
     characterStartIndex: number,
     characterEndIndex: number,
-    enclosingRange?: Range,
-  ): void {
+    enclosingRange: Range | undefined,
+    verseRef: string | undefined,
+    leftContext: string | undefined,
+    rightContext: string | undefined,
+  ): Promise<void> {
     if (!this.allowedCharacterSet.isCharacterAllowed(character)) {
-      this.addDisallowedCharacterWarning(character, characterStartIndex, characterEndIndex, enclosingRange);
+      await this.addDisallowedCharacterWarning(
+        character,
+        characterStartIndex,
+        characterEndIndex,
+        enclosingRange,
+        verseRef,
+        leftContext,
+        rightContext,
+      );
     }
   }
 
-  private addDisallowedCharacterWarning(
+  private async addDisallowedCharacterWarning(
     character: string,
     characterStartIndex: number,
     characterEndIndex: number,
-    enclosingRange?: Range,
+    enclosingRange: Range | undefined,
+    verseRef: string | undefined,
+    leftContext: string | undefined,
+    rightContext: string | undefined,
   ) {
     const code: string = AllowedCharacterIssueFinder.DIAGNOSTIC_CODE;
-    const diagnostic: Diagnostic = this.diagnosticFactory
+    const diagnostic: Diagnostic = await this.diagnosticFactory
       .newBuilder()
       .setCode(code)
       .setSeverity(DiagnosticSeverity.Warning)
@@ -75,6 +98,10 @@ export class AllowedCharacterIssueFinder implements IssueFinder {
           character: character,
         }),
       )
+      .setVerseRef(verseRef)
+      .setContent(character)
+      .setLeftContext(leftContext)
+      .setRightContext(rightContext)
       .build();
 
     this.diagnosticList.addDiagnostic(diagnostic);