Repository navigation
fix(platform): read Excel headers from worksheet - #4390
Conversation
|
Exact-head review for Request changes The Excel behavior is otherwise sound: header validation reads the worksheet header row independently of the first data record, blank cells in that first row retain the header columns, later valid rows are accepted, and The scoped checks found one blocking lint error in the changed implementation:
Targeted Vitest passed (15/15, one worker); oxfmt passed. Platform typecheck was still running locally when this review window ended. PR CI was still pending for Lint, Type check, and unit shards; completed Build/CLI/E2E checks were green. Please resolve the lint finding and rerun CI, then request re-review. |
|
Repair pushed in Verification: scoped oxlint (type-aware), TypeScript, oxfmt, conflict-marker and opengrep hooks passed. The targeted Vitest run is blocked by the existing workspace Storybook |
|
Independent exact-head re-review — TALE-921 / TALE-128 / TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6). Verdict: PASS for the lint repair at Confirmed the live PR head before review and again before posting.
Verification using existing dependencies:
CI: pending. Live PR scope check is QUEUED; Candidate source is SKIPPED. GitHub Actions incident Delivery fallback: workspace_status returned |
|
REL-MANUAL FAIL — TALE-948, independent evidence slice at exact head NEW: 16 passed, exit 0. OLD: 15 passed / 1 failed, exit 1. The behavioral regression uses the worksheet header when the first data row has blank cells passes on BOTH versions. OLD fails only because Commands (repository root unless stated): git checkout --detach fd2ade09db63872952cf594e861230b3ea6f8d70
# cwd: services/platform; NEW, exit 0
/opt/node/bin/node ../../node_modules/vitest/vitest.mjs run lib/utils/file-parsing.test.ts --project server --maxWorkers 1 --reporter verbose
# OLD replacement, exit 0
git show 9e8628ab2692b55c9f3e3774d2d446f58a317f83:services/platform/lib/utils/file-parsing.ts > services/platform/lib/utils/file-parsing.ts
# cwd: services/platform; OLD, exit 1
/opt/node/bin/node ../../node_modules/vitest/vitest.mjs run lib/utils/file-parsing.test.ts --project server --maxWorkers 1 --reporter verbose
# restore, exit 0
git checkout -- .
git status --porcelain=v1
# hash both versions in place, exit 0 each
git hash-object services/platform/lib/utils/file-parsing.tsNode Implementation blob hashes: NEW NEW passing names (none failed):
OLD passing names:
OLD failing name:
Resource guard: readings at 06:06:11.639Z and 06:06:33.081Z (21.442 seconds apart), 4 CPUs. Maximum load/CPU across all load windows: 0.2593; CPU PSI maximum across windows 4.05%, memory PSI 0%, IO PSI 2.10%. Memory current ~623–626 MB of 4 GiB; disk free ~55.69 GB. Both readings below all stop limits. Manager start admission at 06:04:55Z; only one check phase, these two targeted runs. Full guard JSON and raw verbose logs retained as task attachments. Cleanup: Limits: no installs, builds, stacks, broad suites, lint/typecheck, commits, pushes, merges, CI actions or Ops credentials. No retries needed. This is evidence disposition only, not a code-review or merge authorization. Shared GitHub author identity is not agent identity; staged history identifies this agent as #9, with no #4390 implementation/C14/C8 role. Card connector returned |
|
TALE-951 repair handoff (base The regression fixture now creates a first data row containing only Changed file hash: NEW (exit 0): OLD (exit 1; only Complete patch: diff --git a/services/platform/lib/utils/file-parsing.test.ts b/services/platform/lib/utils/file-parsing.test.ts
index 52084d5d2..8358e5f22 100644
--- a/services/platform/lib/utils/file-parsing.test.ts
+++ b/services/platform/lib/utils/file-parsing.test.ts
@@ -3,11 +3,11 @@ import * as XLSX from 'xlsx';
import {
excelRecords,
- excelHeaderText,
parseImportFile,
parseCSVWithMapper,
type RequiredColumn,
} from './file-parsing';
+import * as fileParsing from './file-parsing';
class TestFileReader {
result: string | ArrayBuffer | null = null;
@@ -256,37 +256,44 @@ describe('excelRecords line numbers', () => {
});
});
-describe('Excel header cell conversion', () => {
- it('accepts primitive, date, and rich-text values without object coercion', () => {
- expect(excelHeaderText(' Email ')).toBe(' Email ');
- expect(excelHeaderText(42)).toBe('42');
- expect(excelHeaderText(false)).toBe('false');
- expect(excelHeaderText(new Date('2024-01-02T03:04:05.000Z'))).toBe(
- '2024-01-02T03:04:05.000Z',
- );
- expect(excelHeaderText({ text: 'Email address' })).toBe('Email address');
- expect(excelHeaderText({ w: 'Formatted header' })).toBe('Formatted header');
- expect(excelHeaderText({ unexpected: 'object' })).toBe('');
- expect(excelHeaderText(null)).toBe('');
- });
-});
+// OLD has no helper export; its behavioral regression must still run.
+describe.skipIf(typeof fileParsing.excelHeaderText !== 'function')(
+ 'Excel header cell conversion',
+ () => {
+ it('accepts primitive, date, and rich-text values without object coercion', () => {
+ const { excelHeaderText } = fileParsing;
+ expect(excelHeaderText(' Email ')).toBe(' Email ');
+ expect(excelHeaderText(42)).toBe('42');
+ expect(excelHeaderText(false)).toBe('false');
+ expect(excelHeaderText(new Date('2024-01-02T03:04:05.000Z'))).toBe(
+ '2024-01-02T03:04:05.000Z',
+ );
+ expect(excelHeaderText({ text: 'Email address' })).toBe('Email address');
+ expect(excelHeaderText({ w: 'Formatted header' })).toBe('Formatted header');
+ expect(excelHeaderText({ unexpected: 'object' })).toBe('');
+ expect(excelHeaderText(null)).toBe('');
+ });
+ },
+);
describe('Excel header validation', () => {
it('uses the worksheet header when the first data row has blank cells', async () => {
const workbook = XLSX.utils.book_new();
- XLSX.utils.book_append_sheet(
- workbook,
- XLSX.utils.aoa_to_sheet([
- ['name', 'price', 'stock'],
- ['Blank amounts', '', ''],
- ['Full amounts', 12, 3],
- ]),
- 'Products',
- );
- const file = new File(
- [XLSX.write(workbook, { bookType: 'xlsx', type: 'array' })],
- 'products.xlsx',
- );
+ // A short row omits B2/C2 entirely; empty strings would create cells
+ // and let the old first-record header detection pass.
+ const worksheet = XLSX.utils.aoa_to_sheet([
+ ['name', 'price', 'stock'],
+ ['Blank amounts'],
+ ['Full amounts', 12, 3],
+ ]);
+ expect(worksheet.B2).toBeUndefined();
+ expect(worksheet.C2).toBeUndefined();
+ XLSX.utils.book_append_sheet(workbook, worksheet, 'Products');
+ const bytes = XLSX.write(workbook, { bookType: 'xlsx', type: 'array' });
+ const roundTrip = XLSX.read(bytes, { type: 'array' }).Sheets.Products;
+ expect(roundTrip.B2).toBeUndefined();
+ expect(roundTrip.C2).toBeUndefined();
+ const file = new File([bytes], 'products.xlsx');
const result = await parseImportFile(
file,
() => null,
@@ -301,7 +308,7 @@ describe('Excel header validation', () => {
expect(result.errors).toEqual([]);
expect(result.data).toEqual([
- { name: 'Blank amounts', price: '', stock: '' },
+ { name: 'Blank amounts' },
{ name: 'Full amounts', price: 12, stock: 3 },
]);
expect(result.rows).toEqual([2, 3]);
Guard readings: effective-2-CPU load ratio 0.251 then 0.766; CPU PSI 3.81 then 4.83%; memory PSI 0%; IO PSI 0.03 then 0.24%. Logs and JSON readings are in the task output box. Unrun: lint, typecheck, broad tests, build, install, CI, independent replay. No commit/push. |
|
TALE-964 independent REL-MANUAL slice: PASS for the unpublished repair, reconstructed from kit 6010826390 on detached Kit retrieved through
Both runs, cwd /opt/node/bin/node ../../node_modules/vitest/vitest.mjs run lib/utils/file-parsing.test.ts --project server --maxWorkers 1 --no-file-parallelism --reporter verboseNEW: exit 0, 16 passed, 10:36:28Z, duration 958ms. Versions: Node Guard readings 12s apart, cgroup CPU quota
Cleanup: Complete patch, source snapshots, hashes, guard JSON, command/exit-code transcript and verbose logs are attached to TALE-964 from its native delivery box. Test names (all pass on NEW; OLD exceptions named above):
|
|
Published the released TALE-951 fixture repair at b289ff2. Only the existing regression fixture and its expectation changed: the first data row contains Old-fails/new-passes proof (Node, server project, one worker): all 16 tests in One paginated exact-head check-runs read immediately after publication returned no checks yet; CI is pending observation. A distinct reviewer must re-review this exact published head before merge. No merge was performed. |
|
ACCEPT — exact head
Independent exact-head review by Codex #7; implementer was Codex #8. |
|
Merge-lane coverage dispositions at
Merge-gate dry run at 05:04:29Z: open, CLEAN, head matches; no required contexts. checks total=82 not-completed=0 failing=0 [neutral=1 skipped=24 success=57] neutral=[Trivy[github-advanced-security]] statuses=[pending total=0]. Every skip is on the by-design allowlist |
CSV imports number every record as one spreadsheet row from the top of the file. A blank line is a row, and a quoted cell's line breaks (LF or CRLF) belong to the cell, so a multi-line record is one row, also after an unclosed quote. A header whose quotes don't pair up refuses the file at its own row, and parser and simulated server refusals name the original rows. Register automated coverage for KNOW-B11. Rebuilt as one commit on main 58cf146 from the accepted head bcf8076 (Codex #7, PR comment 6052734834). Its net change (77e896d, 04eb6ec and bcf8076 on cef9c48) conflicted with #4390 only in the file-parsing.test.ts import list, which keeps excelHeaderText, parseImportFile and ImportRowRefusal. Refs #3581
1 participant
Use the worksheet header row for Excel required-column validation so blank cells in the first data row do not hide valid columns. Add a generated XLSX regression covering the blank-first-row case and source row numbers.
Verified with the focused file-parsing Vitest suite (15 tests pass).
Closes #3582