Skip to content

CSV round-trip for corrections on the transcript list views - #86

Merged
vijay-tylertech merged 1 commit into
mainfrom
feat/csv-corrections
Aug 29, 2026
Merged

vijay-tylertech merged 1 commit into
mainfrom
feat/csv-corrections

Conversation

@vijay-tylertech

Copy link
Copy Markdown
Contributor

Corrections are the one part of a review somebody else may be better placed to
write -- a product owner who knows what the answer should have been and is never
going to run a local review server. Export the exchanges, let them fill the fourth
column in whatever they already use, import it back.

Four columns. The first three carry (DO NOT MODIFY) because they are IDENTITY, not
payload: they say which exchange a correction belongs to. Only the Correction is
read back.

Transcript id (DO NOT MODIFY) | Exchange question (DO NOT MODIFY)
Answer given (DO NOT MODIFY) | Correction

MATCHING is transcript id + question text, never row position -- position breaks the
moment someone sorts the sheet or deletes a row they had nothing to say about, and
both are things people do to a spreadsheet without thinking of them as destructive.
An edited question therefore does not rewrite the transcript; it stops matching and
the row is dropped, which is the stated requirement and the reason the header says
so. A question repeated inside one transcript resolves in document order, exchanges
being claimed as they match.

Decisions worth knowing, since neither was specified:

  • A BLANK correction means "nothing supplied", NOT "delete". A spreadsheet
    round-trip drops cells for dull reasons, and an import that wiped corrections on
    a blank would destroy work recoverable from nowhere else. Clearing stays a
    deliberate act in the transcript form.
  • pushed and excluded transcripts are REFUSED, not written. Both states are
    assertions -- content already live in Foundry, conversation out of scope -- and
    quietly editing the body would leave the assertion false.

Selection is no longer restricted to pending rows. The checkbox was disabled on
anything not pending, which was right while bulk-marking was the only bulk action,
but export is a read and refusing to export a reviewed transcript blocks the case
the feature exists for. data-bulkok carries the old rule, the mark button reads it
and says "2 of 5 can be marked reviewed" rather than only greying out, and /bulk
enforces it server-side regardless -- as its own comment already anticipated: "the
disabled checkbox in the UI is advice; this is the rule".

Two things found by testing rather than reasoning:

  • A correction containing <!-- /review:1 --> TERMINATED ITS OWN BLOCK: the stored
    correction was truncated at that point and the remainder leaked into the document
    body as loose prose. Measured -- a five-line correction came back as one word.
    Imported text now has every <!-- neutralised to &lt;!--, which markdown
    renders literally and which cannot terminate anything.
  • Excel guesses the encoding of a .csv without a BOM and guesses wrong on anything
    non-ASCII, so an export would return with mojibake in the questions and then fail
    to match. Written on export (verified EF BB BF in the delivered bytes), stripped
    on import. Import also sniffs the delimiter: European Excel writes semicolons,
    which would otherwise parse as one column and match nothing.

Verified: 10 import cases (happy path, identical, blank, edited question, edited
answer, unknown id, 3 columns, semicolon, headerless, whitespace/case reflow); a
7-exchange transcript whose diff is exactly 7 corrections plus the same needs-triage
repair the form does; duplicate questions mapping in order with the surplus row
reported by line number; pushed and excluded left byte-identical; a correction with
commas, quotes, backslashes, \1, $1, unicode and newlines round-tripping byte-exact;
and the whole flow driven through the real file input in Chrome. Contrast green.

Claude-Session: https://claude.ai/code/session_01VVNYqGza2dFdeWFX7qXnwb

Corrections are the one part of a review somebody else may be better placed to
write -- a product owner who knows what the answer should have been and is never
going to run a local review server. Export the exchanges, let them fill the fourth
column in whatever they already use, import it back.

Four columns. The first three carry (DO NOT MODIFY) because they are IDENTITY, not
payload: they say which exchange a correction belongs to. Only the Correction is
read back.

  Transcript id (DO NOT MODIFY) | Exchange question (DO NOT MODIFY)
  Answer given (DO NOT MODIFY)  | Correction

MATCHING is transcript id + question text, never row position -- position breaks the
moment someone sorts the sheet or deletes a row they had nothing to say about, and
both are things people do to a spreadsheet without thinking of them as destructive.
An edited question therefore does not rewrite the transcript; it stops matching and
the row is dropped, which is the stated requirement and the reason the header says
so. A question repeated inside one transcript resolves in document order, exchanges
being claimed as they match.

Decisions worth knowing, since neither was specified:

  * A BLANK correction means "nothing supplied", NOT "delete". A spreadsheet
    round-trip drops cells for dull reasons, and an import that wiped corrections on
    a blank would destroy work recoverable from nowhere else. Clearing stays a
    deliberate act in the transcript form.
  * `pushed` and `excluded` transcripts are REFUSED, not written. Both states are
    assertions -- content already live in Foundry, conversation out of scope -- and
    quietly editing the body would leave the assertion false.

Selection is no longer restricted to pending rows. The checkbox was disabled on
anything not pending, which was right while bulk-marking was the only bulk action,
but export is a read and refusing to export a reviewed transcript blocks the case
the feature exists for. `data-bulkok` carries the old rule, the mark button reads it
and says "2 of 5 can be marked reviewed" rather than only greying out, and /bulk
enforces it server-side regardless -- as its own comment already anticipated: "the
disabled checkbox in the UI is advice; this is the rule".

Two things found by testing rather than reasoning:

  * A correction containing `<!-- /review:1 -->` TERMINATED ITS OWN BLOCK: the stored
    correction was truncated at that point and the remainder leaked into the document
    body as loose prose. Measured -- a five-line correction came back as one word.
    Imported text now has every `<!--` neutralised to `&lt;!--`, which markdown
    renders literally and which cannot terminate anything.
  * Excel guesses the encoding of a .csv without a BOM and guesses wrong on anything
    non-ASCII, so an export would return with mojibake in the questions and then fail
    to match. Written on export (verified EF BB BF in the delivered bytes), stripped
    on import. Import also sniffs the delimiter: European Excel writes semicolons,
    which would otherwise parse as one column and match nothing.

Verified: 10 import cases (happy path, identical, blank, edited question, edited
answer, unknown id, 3 columns, semicolon, headerless, whitespace/case reflow); a
7-exchange transcript whose diff is exactly 7 corrections plus the same needs-triage
repair the form does; duplicate questions mapping in order with the surplus row
reported by line number; pushed and excluded left byte-identical; a correction with
commas, quotes, backslashes, \1, $1, unicode and newlines round-tripping byte-exact;
and the whole flow driven through the real file input in Chrome. Contrast green.

Claude-Session: https://claude.ai/code/session_01VVNYqGza2dFdeWFX7qXnwb
@vijay-tylertech
vijay-tylertech requested a review from a team as a code owner August 29, 2026 21:48
@vijay-tylertech
vijay-tylertech merged commit 6953f05 into main Aug 29, 2026
1 check passed
@vijay-tylertech
vijay-tylertech deleted the feat/csv-corrections branch August 29, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant