log-client: fix resuming with no matching logs (#206)

* log-client: fix resuming with no matching logs

* log-client: add new resume tests

.changeset/honest-dolls-smash.md

@@ -0,0 +1,5 @@
+---
+'@lightmill/log-client': patch
+---
+
+Fix resuming a log when there are no matching logs found.

packages/log-client/__tests__/main.test.ts

@@ -222,7 +222,9 @@ describe('RunLogger#resumeRun', () => {
       runId: 'test-run',
       experimentId: 'test-experiment',
     });
-    await logger.resumeRun({ resumeAfterLast: 'test-type' });
+    await expect(
+      logger.resumeRun({ resumeAfterLast: 'test-type' }),
+    ).resolves.toEqual({ type: 'test-type', number: 4 });
     await expect(waitForChangeRequests()).resolves.toEqual([
       {
         url: 'https://server.test/api/experiments/test-experiment/runs/test-run',
@@ -250,7 +252,9 @@ describe('RunLogger#resumeRun', () => {
       runId: 'test-run',
       experimentId: 'test-experiment',
     });
-    await logger.resumeRun({ resumeAfterLast: ['test-type1', 'test-type2'] });
+    await expect(
+      logger.resumeRun({ resumeAfterLast: ['test-type1', 'test-type2'] }),
+    ).resolves.toEqual({ type: 'test-type1', number: 3 });
     await expect(waitForChangeRequests()).resolves.toEqual([
       {
         url: 'https://server.test/api/experiments/test-experiment/runs/test-run',
@@ -260,6 +264,35 @@ describe('RunLogger#resumeRun', () => {
     ]);
   });
 
+  it('should resume an existing run even if no matching logs are found', async () => {
+    setServerRuns([
+      {
+        runId: 'test-run',
+        experimentId: 'test-experiment',
+        status: 'running',
+        logs: [
+          { type: 'test-type1', count: 3, lastNumber: 5, pending: 2 },
+          { type: 'test-type2', count: 1, lastNumber: 1, pending: 1 },
+        ],
+      },
+    ]);
+    const logger = new LogClient({
+      apiRoot: 'https://server.test/api',
+      runId: 'test-run',
+      experimentId: 'test-experiment',
+    });
+    await expect(
+      logger.resumeRun({ resumeAfterLast: ['other-type'] }),
+    ).resolves.toBeNull();
+    await expect(waitForChangeRequests()).resolves.toEqual([
+      {
+        url: 'https://server.test/api/experiments/test-experiment/runs/test-run',
+        method: 'PATCH',
+        body: { resumeFrom: 1 },
+      },
+    ]);
+  });
+
   it('should be able to use the runId and experimentId from its parameter', async () => {
     setServerRuns([
       {
@@ -274,11 +307,13 @@ describe('RunLogger#resumeRun', () => {
       },
     ]);
     const logger = new LogClient({ apiRoot: 'https://server.test/api' });
-    await logger.resumeRun({
-      resumeAfterLast: ['test-type1', 'test-type2'],
-      experimentId: 'test-experiment',
-      runId: 'test-run',
-    });
+    await expect(
+      logger.resumeRun({
+        resumeAfterLast: ['test-type1', 'test-type2'],
+        experimentId: 'test-experiment',
+        runId: 'test-run',
+      }),
+    ).resolves.toEqual({ type: 'test-type1', number: 3 });
     await expect(waitForChangeRequests()).resolves.toEqual([
       {
         url: 'https://server.test/api/experiments/test-experiment/runs/test-run',
@@ -297,18 +332,20 @@ describe('RunLogger#resumeRun', () => {
         logs: [
           { type: 'test-type1', count: 3, lastNumber: 5, pending: 2 },
           { type: 'test-type2', count: 1, lastNumber: 1, pending: 1 },
-          { type: 'other-type', count: 3, lastNumber: 7, pending: 0 },
+          { type: 'other-type', count: 4, lastNumber: 8, pending: 0 },
         ],
       },
     ]);
     const logger = new LogClient({
       apiRoot: 'https://server.test/api',
     });
-    await logger.resumeRun({
-      resumeAfterLast: ['test-type1', 'test-type2'],
-      experimentId: 'test-experiment',
-      runId: 'test-run',
-    });
+    await expect(
+      logger.resumeRun({
+        resumeAfterLast: ['test-type1', 'test-type2'],
+        experimentId: 'test-experiment',
+        runId: 'test-run',
+      }),
+    ).resolves.toEqual({ type: 'test-type1', number: 3 });
     await expect(waitForChangeRequests()).resolves.toEqual([
       {
         url: 'https://server.test/api/experiments/test-experiment/runs/test-run',
@@ -318,7 +355,7 @@ describe('RunLogger#resumeRun', () => {
     ]);
   });
 
-  it("should refuse to overwrite the constructor's experimentId", async () => {
+  it('should refuse to resume a run with an experimentId different than the one provided in the constructor', async () => {
     const logger = new LogClient({
       apiRoot: 'https://server.test/api',
       experimentId: 'original-experiment',
@@ -332,7 +369,7 @@ describe('RunLogger#resumeRun', () => {
     ).rejects.toThrowError(Error);
   });
 
-  it("should refuse to overwrite the constructor's runId", async () => {
+  it('should refuse to resume a run with a runId different than the one provided in the constructor', async () => {
     const logger = new LogClient({
       apiRoot: 'https://server.test/api',
       runId: 'original-run',

packages/log-client/src/log-client.ts

@@ -164,8 +164,9 @@ export class LogClient<ClientLog extends Typed & OptionallyDated = AnyLog> {
         .filter((l): l is { type: T } & typeof l =>
           resumeAfterLastSet.has(l.type),
         )
-        .reduce((maxLog, log) =>
-          maxLog.lastNumber > log.lastNumber ? maxLog : log,
+        .reduce(
+          (maxLog, log) => (maxLog.lastNumber > log.lastNumber ? maxLog : log),
+          { lastNumber: 0, count: 0, type: null as null | T },
         );
       await Interface.resumeRun({
         apiRoot: this.#apiRoot,
@@ -177,7 +178,9 @@ export class LogClient<ClientLog extends Typed & OptionallyDated = AnyLog> {
       this.#experimentId = experimentId;
       this.#runStatus = 'running';
       this.#logCount = lastLog.lastNumber;
-      return { type: lastLog.type, number: lastLog.count };
+      return lastLog.type == null
+        ? null
+        : { type: lastLog.type, number: lastLog.count };
     } catch (err) {
       this.#runStatus = 'error';
       throw err;