feat(formats): Add CSV file format support #genai

[rdamazio]

Implement import and export functionality for CSV format checklists as requested in issue #531.

Fixes #531

Rationale:

• Uses established, battle-tested npm libraries (csv-parse and csv-stringify) for CSV operations.
• Uses sync entry points to ensure zero Node.js stream overhead in the browser application.

Summary of changes:

• Added FormatId.CSV and registered CsvFormat in FormatRegistry.
• Added CsvReader for parsing de-normalized CSV tables and metadata into ChecklistFile protobufs.
• Added CsvWriter for serializing ChecklistFile protobufs into formatted CSV files.
• Added comprehensive unit tests in csv-format.spec.ts covering parsing, writing, metadata extraction, round-trip serialization, and error handling.
• Installed csv-parse and csv-stringify packages.
• Created src/model/formats/test.csv representing EXPECTED_CONTENTS.
• Added unit test in csv-format.spec.ts to verify reading test.csv matches EXPECTED_CONTENTS.
• Updated Name field in test string to include a comma.
• Updated assertion to expect "My Airplane Checklists, Main".
• Added test case verifying default values for unpopulated metadata fields when parsing minimal CSV.
• Refactored CsvReader header search to require only minimum columns (Group, Checklist, Type, Text) and map column indices dynamically.
• Made Response, Indent, and Center columns optional when parsing CSV tables.
• Added unit test should parse correctly when optional columns Indent and Center are missing.
• Updated CsvReader row iteration logic to carry over currentGroup.title and currentChecklist.title when group or checklist cells are empty.
• Added unit tests:
  • should parse correctly when Group column value is omitted on subsequent rows
  • should parse correctly when both Group and Checklist column values are omitted on subsequent rows
• Added colIndexToLetters and cellId helper functions in csv-reader.ts.
• Updated CsvReader error messages to report cell references (e.g. "cell A2: invalid/missing...").
• Updated unit test assertions in csv-format.spec.ts to verify cell ID error formatting.

Agent Metadata:
Agent: Antigravity AI Coding Assistant (Gemini 3.6 Flash)
Model: Gemini 3.6 Flash

[zyv]

Just to clarify this before I do anything at all, is code generation with Opus okay or not (I mean in terms of policy, copyright, etc.)?

Also, it’s a non-trivial challenge for me to re-define what review means in the first place for LLM-supported PRs in collaboratively developed projects. I think I kind of figured this out for solo development, but this is a different setting...

[rdamazio]

Treat it as a human-generated change - meaning the human running the AI is responsible for understanding it and making it a good change (no AI slop allowed), and the reviewer is responsible for enforcing code quality and etc too. Does that help?

[zyv]

Treat it as a human-generated change - meaning the human running the AI is responsible for understanding it and making it a good change (no AI slop allowed), and the reviewer is responsible for enforcing code quality and etc too. Does that help?

Alright, thanks - let me give it a try under these premises. I expect to do the following - add a new commit showing what I think the problems are and how I would go about it - and add some evidence for problems solved. My thinking is, it's much easier than to write tons of prose and circle around with AI tools. It would help if you could hold off re-committing until then. Would that be okay with you?

[rdamazio]

Yup, that's fine.

[zyv]

Alright, so this is not actually a review, but more of a rewrite - and it took way more effort than I wished, but there we go. Hopefully, at least I can now take it off my mind :)

In as far as the feature is concerned, I think we don't have much to discuss: it's well-justified, the PR does what it says on the tin and does it properly in as far as I can judge from sane-looking code and passing tests.

The limitations that I thought were worth mentioning explicitly are as follows:

• Categories and completion actions are not supported
• Semicolon-based files cannot be imported
• Column order is fixed
• BOM handling

The first one I addressed in the README, I hope you don't mind my edits.

Regarding the second one, it's trivial to address by re-trying the parsing with a different delimiter. If you are wondering, it's a real issue here over the pond. At first, I was quite confused as to why everything in your PR was completely broken both in terms of Excel and Numbers imports/exports, until I realized that it's the old good CSV delimiter problem.

In as far as BOM handling is concerned, you'll get issues importing files with umlauts in Excel if you don't set it on export. Unfortunately, on top of that, Excel won't import CSVs with non-regional separator (,) without you telling it, but I left , as default.

The column order point you kind of get for free if you switch to a different parsing strategy.

---

Now, in terms of code, I'm afraid I can't get myself to review it in terms of correctness, edge case handling, and expression style, because by now you surely know how my brain is wired. I just get deadlocked on mutable imperative style :(

I have re-written it functionally like my FF/Garmin imports, but I tried to avoid going overboard with reductions and left a couple of places with imperative accumulators. My thinking is, TypeScript is just not the sort of language in which it looks understandable and beautiful. You can go in the direction to a certain extent and it's fewer lines of code, but then you kind of get lost in the confusing syntax and backwards evaluation order.

Apart from the expression style, the biggest issues I had with the original code were the switch tables (if (normalizedKey === ...), if (normalizedType === 'title bar') ..., switch (item.type) ...) and metadata parsing.

In my version, these are simply invertable maps and somewhat type-safer via type aliases. The writer becomes a few trivial maps. The reader, I'd argue, is more understandable than the iterative version, and more testable, although I didn't push that.

I hope this helps, and I wonder what you would make of that.

[zyv]

P.S. I did drop csv-stringify on purpose - feels like one of those packages engineered for supply chain attacks, but that’s my thing. Feel free to revert.

[rdamazio]

it took way more effort than I wished

Let's not do this again then.

Column order is fixed

That wasn't the case before?

one of those packages engineered for supply chain attacks

Not sure what you mean?

it's the old good CSV delimiter problem
it's a real issue here over the pond

You'll have to explain that to me, 'cause I'm clueless here.

In as far as BOM handling is concerned

That we could handle by just adding the marker to the array when creating File, no need to re-implement serialization just for that.

I just get deadlocked on mutable imperative style :(

Anything I can do to help? This is a tiny feature, and it's significantly more complex now 😄

[rdamazio]

(I have a busy week ahead, I'll probably look at your version in detail next weekend)

[rdamazio]

(and btw my reply was not a complaint, sorry if it read that way - just really trying to understand the rationale for your changes)

[zyv]

it took way more effort than I wished

Let's not do this again then.

Oh, I wish I could learn not to get carried away one day, but at this age I might need to face the fact that the likelihood is rather slim.

Column order is fixed

That wasn't the case before?

It was the case before, but after my rewrite, it isn't the case anymore. This was a supporting argument for addressing the columns by name instead of index, which falls out naturally from the functional rewrite.

one of those packages engineered for supply chain attacks

Not sure what you mean?

Unless I'm missing something important (which can totally be the case - I'm not being ironic here), formatting lines is just a matter of a one-liner...

public static formatRow(cells: readonly string[]): string {
  return `${cells.map(CsvUtils._formatCell).join(',')}\r\n`;
}

private static readonly QUOTABLE = /["\r\n,]/;

private static _formatCell(cell: string): string {
  return CsvUtils.QUOTABLE.test(cell) ? `"${cell.replaceAll('"', '""')}"` : cell;
}

which I split into two functions for readability.

If this is really all there is to it, I would prefer this to pulling in a third-party NPM package for it, which comes with a lot of code we don't need, must be regularly updated, the maintainer must be trusted, etc.

There is a tendency in the Node space to generously create packages for common one-liners like this and use them liberally, but many have gained notoriety in recent years, as maintainers either resigned and handed over the maintenance to malicious actors willingly, or got exploited by being tricked to accept malicious pull requests, or even set up such packages as honeypots to activate later, being malicious themselves. Either way, this basically meant that downstream end-user apps got infected all over the world, because someone in the middle of their supply chain, which they weren't even aware of, pulled in an infected one-liner-package.

This is not to say I'm against using libraries. I think using a CSV parsing library is absolutely the right thing to do, since I would expect quite some annoying edge cases there, and wouldn't want to deal with them myself... and maintain the results.

Yet, I've always been conservative about pulling in more libraries, and the latest supply chain dramas only reinforced this attitude. If possible, I want upstream libraries that save more than a few lines of code and have a track record of being professionally maintained over decades.

Having that said, csv-stringify feels exactly like a cleverly devised package engineered for supply chain attacks - everybody wants stuff like that, and nobody wants to carry 3 lines of code, and there might be edge cases, and it has been out for a while... so tempting to reach for it ;) and then get punished by shipping an AI inference miner along with one’s own code <g>

Anyhow, this was all not meant very seriously though, rather more like a tongue-in-cheek remark; I hope this is clearer now - and if you want to serialize via package, just revert it - only I'd be curious about your justification for it.

it's the old good CSV delimiter problem
it's a real issue here over the pond

You'll have to explain that to me, 'cause I'm clueless here.

Okay, let me try to explain in a way that hopefully makes a bit more sense:

In the US, the standard decimal separator is a full stop (.), like in "$10.99". In the EU (in most countries anyways, he-he - why would we agree on one thing for everyone - that would be too dictatorial), the standard decimal separator is , - as in "9,98 €".

Consequently, in the US, a comma is unlikely to collide with decimal numbers, unless you use it as a thousands separator (for which purpose in the EU, you can conveniently use the freed-up full stop, if you use a comma as a decimal separator instead). But spreadsheets don't output thousand separators by default, and not in CSV anyways.

Therefore, in the US, CSV is what it says on the tin - files with lines of comma-separated fields. Occasional entries containing commas must be quoted, but that's about it.

However, in the EU, the spreadsheet writers quickly figured that they'd have to quote every number if they wanted to keep commas as a default separator. What's the problem? There's no problem in as far as I'm concerned.

But some cosmic genius, just like the null-pointer dude, decided that it was a much better idea to silently fall back to a different separator instead, based on regional settings (of all things), and that would be a semicolon (;), because the likelihood is very small that it's going to clash with something (other than... well... a semicolon) and result in pervasive quoting. And this brilliant decision, just like the null-pointer one, caused endless pain and frustration ever since, and will keep causing it till the end of time.

So in the EU, every spreadsheet program (Excel, Numbers, you name it) by default actually saves semicolon-separated files instead of CSV, and NO, you can't specify a delimiter in the advanced settings. See, I'm not kidding - this is not a joke; I'm being dead serious now:

The programs are also able to import their CSV files, which are actually SSV files. But if you point them to a real CSV, you get this:

So you have to re-adjust the import settings manually to get it to work if you are a brilliant software engineer like I am and can see it through... or complain to ChatGPT that the file is corrupted otherwise. And sure enough, you would get the same trying to import my CSV files.

Because in the age of AI, it would be too user-friendly and make too much sense to just sniff out the format and do the right thing if you can, right?

Anyhow, this means that everything that came from your PR initially looked broken to me. I quickly realized that I had to change the import settings - it's been a while since I've used a spreadsheet, but it took me some time to understand why the upload doesn't work.

After I fixed the exception swallowing, the reason became apparent. You only expect genuine comma-separated files, as in files with fields separated with commas like the name says, and I can't blame you for that. Not an old good European semicolon-separated flavor of comma-separated files.

So I fixed that by trying to import twice with different delimiter settings, which seems to me the easiest and safest fix - although maybe not a very performant one, but then I think this is not a context where it would matter. I left the export alone though, as the import can be adjusted in the spreadsheet program as illustrated above.

Does that help?

In as far as BOM handling is concerned

That we could handle by just adding the marker to the array when creating File, no need to re-implement serialization just for that.

Here I absolutely concur, this was meant as an independent observation and not a justification for a rewrite - the only statement that was intended to justify that was the brain wiring issue.

I just get deadlocked on mutable imperative style :(

Anything I can do to help? This is a tiny feature, and it's significantly more complex now 😄

I'm not sure about that, do you happen to be a licensed therapist :) ? The issue seems to be that to me it now looks much simpler and more understandable, most probably just as the imperative version looks simpler and clearer to you, otherwise you wouldn't have posted it.

I'm not sure what can be done about this other than brain rewiring, but I have tons of pseudo-rational arguments why my wiring is objectively better (while reading, reasoning about pure methods and immutable state requires less energy and is less error-prone than in-line symbolic execution to confirm data flow and asses side effects), so this is probably where the 'therapist' part comes in... to show that maybe it's not all that objectively better, and my arguments have more emotional that rational nature, etc. - but also I'm not sure I want to get rewired :/

[zyv]

(and btw my reply was not a complaint, sorry if it read that way - just really trying to understand the rationale for your changes)

No worries, this is exactly how I understood your message, and I hope you can understand my attempt at an explanation in kind.

[zyv]

(I have a busy week ahead, I'll probably look at your version in detail next weekend)

P.S. It's not that the code can't be massaged into making it even nicer, clearer, and more understandable - it absolutely can. It's just that even with my undesirable getting carried away characteristics, I had to stop at some point, because no amount of massage makes sense if you wouldn't be interested in a different approach in principle.