Review brief

Intermediate35 min

A workspace-administration service is adding bulk member import for organization owners. The request layer supplies a validated delimited file, and the importer must protect the workspace from duplicate identities and partial results while returning useful source-row errors.

Behavioral contract

The request layer supplies UTF-8 text with the exact display_name|address header, one unquoted member per following line, no embedded delimiters or newlines, and syntactically valid non-empty fields.
At least one and at most 500 data rows are accepted. A larger input returns a file-too-large error at row 0 without repository access.
Trim display names. Normalize address identity by trimming and lowercasing before every in-file or existing-member comparison and before insertion.
Any normalized address already in the workspace or repeated anywhere in the file makes the entire import invalid. For an in-file duplicate, the second and each later occurrence are offending. Report duplicate-address on each offending physical file row; the header is row 1 and the first member is row 2.
findExistingAddresses accepts normalized addresses and returns normalized stored identities.
Complete parsing, normalization, and validation for the whole file before writing. If any error exists, make no insert call; otherwise make one insertMany call, which is atomic for its complete input.

Review instructions

1Trace parsing, normalization, validation, and writes across the complete file.
2Anchor each finding to the narrowest useful changed line and explain the data-integrity impact.
3Recommend a direction rather than writing replacement code.
4Prioritize no more than five findings.

Learning objectives

• Normalize before identity comparisons
• Preserve invariants across chunk boundaries
• Review atomicity in validation-then-write workflows

src/member-import-types.tsTypeScript

@@ -0,0 +1,18 @@

1+export interface NewMember {

2+ displayName: string;

3+ address: string;

4+}

6+export interface MemberImportError {

7+ row: number;

8+ code: "duplicate-address" | "file-too-large";

9+}

10+

11+export type MemberImportResult =

12+ | { ok: true; imported: number }

13+ | { ok: false; errors: MemberImportError[] };

14+

15+export interface MemberRepository {

16+ findExistingAddresses(addresses: readonly string[]): Promise<readonly string[]>;

17+ insertMany(members: readonly NewMember[]): Promise<void>;

18+}

Reference solution

The findings below are ordered by priority.

Findings

1. Repository writes begin before the whole file is known to be valid

Location: src/member-import.ts:49-73

Each clean chunk is inserted immediately, while duplicate validation for later chunks has not happened yet. If a later chunk contains an error, the function returns ok: false after earlier members have already been committed. A rejection from a later insertMany call likewise leaves earlier calls committed, so both validation and write failures violate the all-or-nothing guarantee.

Parse, normalize, and validate the entire file without writes first. Only when no errors remain should the importer make one atomic insertMany call for the complete member set.

2. In-file identity checks happen before address normalization

Location: src/member-import.ts:26-30,54-59

The duplicate set compares raw row.address strings, and normalization happens only when a clean chunk is prepared for insertion. Addresses such as Alice@Example.com and alice@example.com therefore look different during validation but become the same stored identity.

Normalize every address before any uniqueness comparison, retain the source row alongside it, and use that normalized value for both validation and insertion.

3. Existing-member lookup receives unnormalized addresses

Location: src/member-import.ts:43-45

The repository lookup is called with the raw parsed strings even though its interface expects normalized identities and existing addresses are stored normalized. A case or surrounding-space variation can bypass the existing-member check and then collide when its normalized form is inserted.

Send the full set of normalized addresses to findExistingAddresses and compare its normalized results against the corresponding source rows.

4. The duplicate set is reset at every chunk boundary

Location: src/member-import.ts:49-59

seenAddresses is created inside the chunk loop. Even exact duplicate strings are missed when the first occurs in one 100-row chunk and the second occurs in another, so chunk placement changes whether the same file is accepted.

Maintain one normalized-address set for the complete input while validating all rows before writing.

5. Error rows are off by one because the header is not counted

Location: src/member-import.ts:20-23

The first data line is assigned row 1 even though the display_name|address header already occupies physical row 1. Duplicate errors therefore point administrators to the line above the offending member.

Assign data-row numbers as index + 2; continue reserving row 0 for the file-level size error.

Reasonable non-findings

The 500-data-row maximum is enforced before repository access.
Trimming and lowercasing addresses is the required normalization; the defect is that identity checks happen earlier.
Trimming display names before insertion is correct.
Splitting on the fixed delimiter is acceptable because the request layer guarantees the documented unquoted input format.
A repository insertMany call is atomic as stated; atomicity is lost because the importer makes separate calls before validation finishes.