feat(sp35): Upload page auto-detects 837P vs 835 from file content
Layer A of the SP35 defense-in-depth fix. Before SP35 the dropdown
silently defaulted to '837p' and never changed when a file was dropped
on the page — uploading an 835 file routed it to /api/parse-837 which
(prior to SP35 Task 2) silently persisted an empty batch.
The change:
1. New pure helper src/lib/x12-detect.ts:
- detectKindFromText(text) reads the first ~4KB and returns the
DetectedKind ('837p' | '835' | '999' | '277ca' | 'ta1' | 'unknown')
by matching the ST01 segment (or the bare TA1 segment for the
no-ST envelope). Cheap substring scan; never invokes tokenize().
- detectKindFromFile(file) is the File-aware wrapper used by the UI.
- detectedKindToParsedBatchKind maps the DetectedKind to the kind
the Upload dropdown supports. Returns null for 999/277CA/TA1 so
the UI can surface a clean 'this file isn't supported here' hint.
2. Upload.tsx: pickFile is now async and reads the file before storing
it. If the detected kind differs from the dropdown's current value,
it switches the dropdown and toasts a hint. If the detected kind is
999/277CA/TA1 (Upload doesn't ingest those), it shows an error toast.
20 new tests in src/lib/x12-detect.test.ts cover the 6 DetectedKind
paths, the File wrapper, case-insensitivity, garbage input, the
ST*8370 false-positive guard, and the detectedKindToParsedBatchKind
mapping.
This commit is contained in:
Nora
@@ -0,0 +1,148 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
detectKindFromText,
|
||||
detectKindFromFile,
|
||||
detectedKindToParsedBatchKind,
|
||||
} from "./x12-detect";
|
||||
|
||||
// Minimal X12 envelopes — just enough to exercise the ST01 match.
|
||||
// Real fixtures live in backend/tests/fixtures; we use synthetic ones
|
||||
// here because the frontend doesn't need the full file content, just
|
||||
// the first ~4KB.
|
||||
|
||||
const ISA_837P = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HC*SENDER*RECEIVER*20260706*1937*1*X*005010X222A1~" +
|
||||
"ST*837*0001*005010X222A1~"
|
||||
);
|
||||
const ISA_835 = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HP*SENDER*RECEIVER*20260706*1937*1*X*005010X221A1~" +
|
||||
"ST*835*0001~"
|
||||
);
|
||||
const ISA_999 = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*FA*SENDER*RECEIVER*20260706*1937*1*X*005010X231A1~" +
|
||||
"ST*999*0001~"
|
||||
);
|
||||
const ISA_277CA = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
|
||||
"ST*277*0001*005010X214~"
|
||||
);
|
||||
const ISA_277CA_QUALIFIED = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
|
||||
"ST*277CA*0001*005010X214~"
|
||||
);
|
||||
const TA1_FILE = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260520*1750*^*00501*000000001*0*P*:~" +
|
||||
"TA1*000000001*20260520*1750*A*000*20260520~" +
|
||||
"IEA*1*000000001~"
|
||||
);
|
||||
|
||||
describe("detectKindFromText", () => {
|
||||
it("detects ST*837* as 837p", () => {
|
||||
expect(detectKindFromText(ISA_837P)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects ST*837P* as 837p (professional qualifier)", () => {
|
||||
const withQualifier = ISA_837P.replace("ST*837*", "ST*837P*");
|
||||
expect(detectKindFromText(withQualifier)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects ST*835* as 835", () => {
|
||||
expect(detectKindFromText(ISA_835)).toBe("835");
|
||||
});
|
||||
|
||||
it("detects ST*999* as 999", () => {
|
||||
expect(detectKindFromText(ISA_999)).toBe("999");
|
||||
});
|
||||
|
||||
it("detects ST*277* as 277ca", () => {
|
||||
expect(detectKindFromText(ISA_277CA)).toBe("277ca");
|
||||
});
|
||||
|
||||
it("detects ST*277CA* as 277ca (qualified form)", () => {
|
||||
expect(detectKindFromText(ISA_277CA_QUALIFIED)).toBe("277ca");
|
||||
});
|
||||
|
||||
it("detects TA1 envelope as ta1 (no ST, bare TA1 segment)", () => {
|
||||
expect(detectKindFromText(TA1_FILE)).toBe("ta1");
|
||||
});
|
||||
|
||||
it("returns unknown for garbage input", () => {
|
||||
expect(detectKindFromText("not edi at all")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("returns unknown for an empty string", () => {
|
||||
expect(detectKindFromText("")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("is case-insensitive on ST segment", () => {
|
||||
// Real X12 is uppercase, but be lenient — files from various tools
|
||||
// sometimes have mixed case.
|
||||
expect(detectKindFromText(ISA_835.toLowerCase())).toBe("835");
|
||||
});
|
||||
|
||||
it("does not false-positive on ST*8370 (5 chars after ST*)", () => {
|
||||
// Catches a regression where the matcher would substring-match too
|
||||
// eagerly (e.g. matching ST*837 against ST*8370*). The require is
|
||||
// that the char after the digits is `*`, not another digit.
|
||||
const noise = "ISA*00*...*ST*8370*0001~";
|
||||
expect(detectKindFromText(noise)).toBe("unknown");
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectedKindToParsedBatchKind", () => {
|
||||
it("maps 837p → 837p", () => {
|
||||
expect(detectedKindToParsedBatchKind("837p")).toBe("837p");
|
||||
});
|
||||
|
||||
it("maps 835 → 835", () => {
|
||||
expect(detectedKindToParsedBatchKind("835")).toBe("835");
|
||||
});
|
||||
|
||||
it("returns null for 999 (Upload page doesn't ingest 999s)", () => {
|
||||
expect(detectedKindToParsedBatchKind("999")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for 277ca", () => {
|
||||
expect(detectedKindToParsedBatchKind("277ca")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for ta1", () => {
|
||||
expect(detectedKindToParsedBatchKind("ta1")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for unknown", () => {
|
||||
expect(detectedKindToParsedBatchKind("unknown")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectKindFromFile", () => {
|
||||
it("detects 837p from a File blob", async () => {
|
||||
const file = new File([ISA_837P], "test.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects 835 from a File blob (the original SP35 repro)", async () => {
|
||||
// The exact scenario the user hit: drop an 835 file on the Upload
|
||||
// page while the dropdown still says "837p". The helper must
|
||||
// return "835" so the UI can switch the dropdown before the user
|
||||
// hits Parse.
|
||||
const file = new File([ISA_835], "tp11525703-835.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("835");
|
||||
});
|
||||
|
||||
it("returns unknown on an empty file (no crash)", async () => {
|
||||
const file = new File([""], "empty.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("unknown");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* X12 file-kind auto-detection helper.
|
||||
*
|
||||
* SP35: the Upload page used to default to "837p" silently — dropping an 835
|
||||
* file on the page while the dropdown still said "837p" routed the file to
|
||||
* /api/parse-837, which silently persisted an empty batch. Layer A of the
|
||||
* defense-in-depth fix is to detect the kind from the file's first few KB
|
||||
* and switch the dropdown automatically before the user clicks Parse.
|
||||
*
|
||||
* Layer B is the server-side envelope check (cyclone.api._transaction_set_id_from_segments).
|
||||
* This helper is purely advisory: if it returns null, the UI keeps whatever
|
||||
* the user picked and the server guard rejects with a clean 400 if it's wrong.
|
||||
*
|
||||
* Detection is intentionally cheap: read the first ~4KB and look for the ST
|
||||
* segment (or the bare TA1 segment, which has no ST envelope). The full file
|
||||
* is never loaded — X12 envelopes are always within the first ISA segment.
|
||||
*/
|
||||
|
||||
import type { ParsedBatchKind } from "@/types";
|
||||
|
||||
/** Number of bytes to read from the start of the file for detection. */
|
||||
const DETECT_BYTES = 4096;
|
||||
|
||||
export type DetectedKind = ParsedBatchKind | "999" | "277ca" | "ta1" | "unknown";
|
||||
|
||||
export function detectKindFromText(text: string): DetectedKind {
|
||||
// Strip whitespace and look for the ST segment.
|
||||
// X12 envelopes are always within the first segment, so we don't need
|
||||
// to scan far. The segment terminator is `~` (per the ISA segment,
|
||||
// which we also don't need to parse here — we only care which kind of
|
||||
// payload follows).
|
||||
//
|
||||
// We do a simple substring search rather than full tokenization because:
|
||||
// - Detection must run before the user even knows if their file is
|
||||
// valid X12. If we ran `tokenize()` and the file has a malformed
|
||||
// ISA, we'd throw and the UI couldn't show a helpful message.
|
||||
// - A simple prefix match on `ST*<kind>*` is enough to disambiguate
|
||||
// every kind the Upload page might receive. False positives are
|
||||
// caught by the server-side guard.
|
||||
const upper = text.toUpperCase();
|
||||
|
||||
// 277CA uses ST*277* or ST*277CA* (per parse_277ca.py line 13 comment).
|
||||
// Check 277CA before 277 because ST*277CA contains "277".
|
||||
if (upper.includes("ST*277CA*")) return "277ca";
|
||||
if (upper.includes("ST*277*")) return "277ca";
|
||||
|
||||
// 837P accepts ST*837* or ST*837P* (the trailing P is the professional
|
||||
// claim qualifier, but the ISA envelope alone tells you it's a
|
||||
// professional file). Check 837P before 837 for the same reason.
|
||||
if (upper.includes("ST*837P*")) return "837p";
|
||||
if (upper.includes("ST*837*")) return "837p";
|
||||
|
||||
// 835 has ST*835* — no other qualifier in common use.
|
||||
if (upper.includes("ST*835*")) return "835";
|
||||
|
||||
// 999 ACK has ST*999* — no qualifier.
|
||||
if (upper.includes("ST*999*")) return "999";
|
||||
|
||||
// TA1 has no ST envelope. The interchange-ack segment is the bare TA1*
|
||||
// immediately after ISA/IEA. Match the segment header.
|
||||
if (/\bTA1\*/.test(upper)) return "ta1";
|
||||
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
/**
|
||||
* Browser-side helper: read the first DETECT_BYTES from a File and detect
|
||||
* its kind. Returns a Promise so it composes naturally with FileReader /
|
||||
* Blob.slice. Returns "unknown" on read error so the caller can keep the
|
||||
* user's current selection and let the server guard surface a clean 400.
|
||||
*/
|
||||
export async function detectKindFromFile(file: File): Promise<DetectedKind> {
|
||||
try {
|
||||
const blob = file.slice(0, DETECT_BYTES);
|
||||
const text = await blob.text();
|
||||
return detectKindFromText(text);
|
||||
} catch {
|
||||
return "unknown";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a DetectedKind to the ParsedBatchKind the Upload dropdown supports.
|
||||
* Returns null for kinds the Upload page can't ingest (999, 277CA, TA1) —
|
||||
* the UI then surfaces a "this file isn't supported here" hint instead of
|
||||
* silently misrouting it. The server-side guards in cyclone.api still
|
||||
* catch any escape.
|
||||
*/
|
||||
export function detectedKindToParsedBatchKind(kind: DetectedKind): ParsedBatchKind | null {
|
||||
switch (kind) {
|
||||
case "837p":
|
||||
return "837p";
|
||||
case "835":
|
||||
return "835";
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user