JsonlReporter.append: spread merge silently lets InvokeResult fields overwrite same-named Prompt fields

Open Beginner friendly
#74 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
74/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Quiet
Tech stack
typescript
Domain
tooling

Research direction

Start at JsonlReporter.append in reporters/jsonl and compare its current spread serialization with the Prompt and InvokeResult shapes described here. Make the JSONL record fields explicit for success and failure while preserving the existing order and optional structuredOutput; done when output matches the current schema and future same-name fields cannot be silently merged.

Written by the indexing model from the issue text.

Description

bug S4

Observed behavior

JsonlReporter.append serializes the JSONL record by spreading both arguments into a single object:

async append(prompt: Prompt, result: InvokeResult): Promise<void> {
  const output = `${JSON.stringify({ ...prompt, ...result })}\n`;
  await appendFile(this.#path, output);
}

Because result is spread after prompt, any future field name that exists on both types is silently overwritten by the value from result. There is no compile-time guard against this collision; TypeScript happily merges the two open-ended object types and the conflicting property just disappears from the output.

Today there is no actual collision: Prompt is { id, prompt } and InvokeResult carries { status, output | reason, structuredOutput? }. So the bug is currently latent. The concern is that adding a field to either type (e.g. a status flag on Prompt, or a prompt echo on a future InvokeResult variant) silently changes the on-disk schema of every JSONL report ever written, with no test failure and no type error.

Expected behavior

The serialized record should pick fields explicitly, so that the JSONL schema is documented in code and a future field collision either fails the build or has to be resolved deliberately. Something like:

const record =
  result.status === 'success'
    ? {
        id: prompt.id,
        prompt: prompt.prompt,
        status: result.status,
        output: result.output,
        ...(result.structuredOutput !== undefined
          ? { structuredOutput: result.structuredOutput }
          : {}),
      }
    : {
        id: prompt.id,
        prompt: prompt.prompt,
        status: result.status,
        reason: result.reason,
      };
const output = `${JSON.stringify(record)}\n`;

This also keeps the existing field order (id, prompt, status, output/reason, structuredOutput) which is what the current spread already produces, so the on-disk format is unchanged.

Minimal reproduction (illustrating the latent collision)

import { JsonlReporter } from 'loop-the-loop/reporters/jsonl';

const reporter = await JsonlReporter.create({ outputDir: '.', jobName: 'demo' });

// If a future Prompt subtype gains a `status` field, it is silently dropped:
await reporter.append(
  { id: 'a', prompt: 'p', status: 'prompt-was-fresh' } as any,
  { status: 'success', output: 'ok' },
);
// File contains: {"id":"a","prompt":"p","status":"success","output":"ok"}
//                                        ^^^ prompt's status was silently overwritten

Severity

S4. There is no current collision, no observable bug today, and the JSONL output is correct for the existing types. Filing as a maintainability / robustness cleanup so the merge does not become a silent schema-changing bug the next time either type evolves.

Dominant language
TypeScript
Stars
2
Forks
1
PR merge metrics
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from joewalker/loop-the-loop

All issues in joewalker/loop-the-loop

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.