fix(section16): preserve null vs 0 for empty numeric leaves on Forms 3/4/5 (#116 follow-up)

src/sec/forms/insider-trading/OwnershipDocument.schema.ts

@@ -22,9 +22,14 @@ const VALUE_STRING = Type.Object({
   value: Type.Optional(Type.String()),
 });
 
-// A `{ value }` wrapper around a numeric leaf. Convert coerces "10000" -> 10000.
+// A `{ value }` wrapper around a numeric leaf. The inner value is kept as a
+// raw string and coerced in storage. Typing it as Type.Number() would let
+// Value.Convert turn an empty element (`<transactionShares><value/></transactionShares>`,
+// which parses to "") into a fabricated 0, indistinguishable from a real zero
+// and silently corrupting transaction and holding data. Storage's `num()`
+// helper maps "" -> null and parses populated strings to numbers.
 const VALUE_NUMBER = Type.Object({
-  value: Type.Optional(Type.Number()),
+  value: Type.Optional(Type.String()),
 });
 
 const ISSUER_TYPE = Type.Object({

src/sec/forms/insider-trading/OwnershipDocument.storage.test.ts

@@ -221,4 +221,132 @@ describe("OwnershipDocument storage (Forms 3/4/5)", () => {
     const persons = await new PersonObservationRepo().listByAccession(accession);
     expect(persons.some((p) => p.last_name === "Chung Chih-Hsiao")).toBe(true);
   });
+
+  it("stores null (not 0) for an empty transactionShares element on a Form 4", async () => {
+    const accession = "0001493152-26-025476";
+    const xml = readFileSync(
+      join(__dirname, "mock_data", "form-4", "000149315226025476-primary_doc.xml"),
+      "utf-8"
+    );
+    const doc = await Form_4.parse("4", xml);
+    // Simulate a filing that emits an empty <transactionShares><value/></transactionShares>.
+    const nonDerivTxn = doc.nonDerivativeTable!.nonDerivativeTransaction![0];
+    nonDerivTxn.transactionAmounts!.transactionShares!.value = "";
+    await processOwnershipForm({
+      cik: 1828673,
+      file_number: "",
+      accession_number: accession,
+      filing_date: "2026-05-27",
+      primary_doc: "x.xml",
+      form: "4",
+      doc,
+    });
+
+    const txns = await repo.getTransactions(accession);
+    const nonDeriv = txns.find((t) => !t.is_derivative)!;
+    expect(nonDeriv.shares).toBeNull();
+    // A populated sibling field still coerces to its real number.
+    expect(nonDeriv.price_per_share).toBe(1.405);
+  });
+
+  it("stores null (not 0) for an empty transactionPricePerShare element", async () => {
+    const accession = "0001493152-26-025476";
+    const xml = readFileSync(
+      join(__dirname, "mock_data", "form-4", "000149315226025476-primary_doc.xml"),
+      "utf-8"
+    );
+    const doc = await Form_4.parse("4", xml);
+    const nonDerivTxn = doc.nonDerivativeTable!.nonDerivativeTransaction![0];
+    nonDerivTxn.transactionAmounts!.transactionPricePerShare!.value = "";
+    await processOwnershipForm({
+      cik: 1828673,
+      file_number: "",
+      accession_number: accession,
+      filing_date: "2026-05-27",
+      primary_doc: "x.xml",
+      form: "4",
+      doc,
+    });
+
+    const txns = await repo.getTransactions(accession);
+    const nonDeriv = txns.find((t) => !t.is_derivative)!;
+    expect(nonDeriv.price_per_share).toBeNull();
+    expect(nonDeriv.shares).toBe(177936);
+  });
+
+  it("stores null (not 0) for an empty sharesOwnedFollowingTransaction on a holding", async () => {
+    const accession = "0000950103-26-007758";
+    const xml = readFileSync(
+      join(__dirname, "mock_data", "form-3-a", "000095010326007758-primary_doc.xml"),
+      "utf-8"
+    );
+    const doc = await Form_3.parse("3/A", xml);
+    const nonDerivHold = doc.nonDerivativeTable!.nonDerivativeHolding![0];
+    nonDerivHold.postTransactionAmounts!.sharesOwnedFollowingTransaction!.value = "";
+    await processOwnershipForm({
+      cik: 1122411,
+      file_number: "",
+      accession_number: accession,
+      filing_date: "2026-05-27",
+      primary_doc: "x.xml",
+      form: "3/A",
+      doc,
+    });
+
+    const holdings = await repo.getHoldings(accession);
+    const nonDeriv = holdings.find((h) => !h.is_derivative)!;
+    expect(nonDeriv.shares_owned_following).toBeNull();
+  });
+
+  it("stores null (not 0) for an empty derivative conversionOrExercisePrice", async () => {
+    const accession = "0000950103-26-007758";
+    const xml = readFileSync(
+      join(__dirname, "mock_data", "form-3-a", "000095010326007758-primary_doc.xml"),
+      "utf-8"
+    );
+    const doc = await Form_3.parse("3/A", xml);
+    const derivHold = doc.derivativeTable!.derivativeHolding![0];
+    derivHold.conversionOrExercisePrice!.value = "";
+    await processOwnershipForm({
+      cik: 1122411,
+      file_number: "",
+      accession_number: accession,
+      filing_date: "2026-05-27",
+      primary_doc: "x.xml",
+      form: "3/A",
+      doc,
+    });
+
+    const holdings = await repo.getHoldings(accession);
+    const deriv = holdings.filter((h) => h.is_derivative);
+    expect(deriv[0].conversion_or_exercise_price).toBeNull();
+    // The other derivative holding still carries its real price.
+    expect(deriv[1].conversion_or_exercise_price).not.toBeNull();
+  });
+
+  it("stores null (not 0) for an empty underlyingSecurityShares on a derivative transaction", async () => {
+    const accession = "0001493152-26-025476";
+    const xml = readFileSync(
+      join(__dirname, "mock_data", "form-4", "000149315226025476-primary_doc.xml"),
+      "utf-8"
+    );
+    const doc = await Form_4.parse("4", xml);
+    const derivTxn = doc.derivativeTable!.derivativeTransaction![0];
+    derivTxn.underlyingSecurity!.underlyingSecurityShares!.value = "";
+    await processOwnershipForm({
+      cik: 1828673,
+      file_number: "",
+      accession_number: accession,
+      filing_date: "2026-05-27",
+      primary_doc: "x.xml",
+      form: "4",
+      doc,
+    });
+
+    const txns = await repo.getTransactions(accession);
+    const deriv = txns.find((t) => t.is_derivative)!;
+    expect(deriv.underlying_security_shares).toBeNull();
+    // A populated sibling field still coerces to its real number.
+    expect(deriv.conversion_or_exercise_price).toBe(1.28);
+  });
 });

src/sec/forms/insider-trading/OwnershipDocument.storage.ts

@@ -5,35 +5,35 @@
  */
 
 import { globalServiceRegistry } from "workglow";
-import { AddressRepo } from "../../../storage/address/AddressRepo";
-import type { AddressImport } from "../../../storage/address/AddressNormalization";
-import { hasCompanyEnding } from "../../../storage/company/CompanyNormalization";
-import { isBadPersonField } from "../../../types/edgar/bad-data";
-import { parseCikSafely } from "../../../util/parseCik";
+import { CompanyResolver } from "../../../resolver/CompanyResolver";
 import { EntityObserver } from "../../../resolver/EntityObserver";
 import { PersonResolver } from "../../../resolver/PersonResolver";
-import { CompanyResolver } from "../../../resolver/CompanyResolver";
-import { PersonObservationRepo } from "../../../storage/observation/PersonObservationRepo";
-import { CompanyObservationRepo } from "../../../storage/observation/CompanyObservationRepo";
-import { PersonIdentityLinkRepo } from "../../../storage/canonical/PersonIdentityLinkRepo";
-import { CompanyIdentityLinkRepo } from "../../../storage/canonical/CompanyIdentityLinkRepo";
-import { CanonicalPersonRepo } from "../../../storage/canonical/CanonicalPersonRepo";
-import { CanonicalCompanyRepo } from "../../../storage/canonical/CanonicalCompanyRepo";
-import { CanonicalPersonAliasRepo } from "../../../storage/canonical/CanonicalPersonAliasRepo";
+import type { AddressImport } from "../../../storage/address/AddressNormalization";
+import { AddressRepo } from "../../../storage/address/AddressRepo";
+import { CanonicalCompanyAddressRepo } from "../../../storage/canonical/CanonicalCompanyAddressRepo";
 import { CanonicalCompanyAliasRepo } from "../../../storage/canonical/CanonicalCompanyAliasRepo";
+import { CanonicalCompanyPhoneRepo } from "../../../storage/canonical/CanonicalCompanyPhoneRepo";
+import { CanonicalCompanyRepo } from "../../../storage/canonical/CanonicalCompanyRepo";
 import { CanonicalPersonAddressRepo } from "../../../storage/canonical/CanonicalPersonAddressRepo";
+import { CanonicalPersonAliasRepo } from "../../../storage/canonical/CanonicalPersonAliasRepo";
 import { CanonicalPersonPhoneRepo } from "../../../storage/canonical/CanonicalPersonPhoneRepo";
-import { CanonicalCompanyAddressRepo } from "../../../storage/canonical/CanonicalCompanyAddressRepo";
-import { CanonicalCompanyPhoneRepo } from "../../../storage/canonical/CanonicalCompanyPhoneRepo";
-import { COMPONENT_VERSION_REPOSITORY_TOKEN } from "../../../storage/versioning/ComponentVersionSchema";
-import { VersionRegistry } from "../../../storage/versioning/VersionRegistry";
-import { getActiveSlot } from "../../../storage/versioning/getActiveSlot";
-import { formToExtractorId } from "../../../storage/versioning/extractorIds";
+import { CanonicalPersonRepo } from "../../../storage/canonical/CanonicalPersonRepo";
+import { CompanyIdentityLinkRepo } from "../../../storage/canonical/CompanyIdentityLinkRepo";
+import { PersonIdentityLinkRepo } from "../../../storage/canonical/PersonIdentityLinkRepo";
+import { hasCompanyEnding } from "../../../storage/company/CompanyNormalization";
+import { CompanyObservationRepo } from "../../../storage/observation/CompanyObservationRepo";
+import { PersonObservationRepo } from "../../../storage/observation/PersonObservationRepo";
 import { Section16Repo } from "../../../storage/section16/Section16Repo";
 import type {
   Section16Holding,
   Section16Transaction,
 } from "../../../storage/section16/Section16Schema";
+import { COMPONENT_VERSION_REPOSITORY_TOKEN } from "../../../storage/versioning/ComponentVersionSchema";
+import { VersionRegistry } from "../../../storage/versioning/VersionRegistry";
+import { formToExtractorId } from "../../../storage/versioning/extractorIds";
+import { getActiveSlot } from "../../../storage/versioning/getActiveSlot";
+import { isBadPersonField } from "../../../types/edgar/bad-data";
+import { parseCikSafely } from "../../../util/parseCik";
 import type { OwnershipDocument } from "./OwnershipDocument.schema";
 
 // EDGAR ownership flags appear as "1"/"0" (X0609) or "true"/"false" (X0607).
@@ -51,12 +51,15 @@ function str(field: { value?: string } | string | undefined): string | null {
   return v === undefined || v === null || String(v).trim() === "" ? null : String(v).trim();
 }
 
-// Unwrap a `{ value }` leaf to a finite number, or null.
-function num(field: { value?: number | string } | string | undefined): number | null {
+// Unwrap a `{ value }` leaf to a finite number, or null. The schema types the
+// inner value as a string so that an empty XML element (parsed as "") survives
+// Value.Convert intact and reaches this helper, which maps "" -> null. If we
+// typed it as a number, Value.Convert would fabricate a 0 here instead.
+function num(field: { value?: string } | string | undefined): number | null {
   if (field === undefined || field === null || typeof field === "string") return null;
   const v = field.value;
-  if (v === undefined || v === null || String(v).trim() === "") return null;
-  const n = typeof v === "number" ? v : Number(v);
+  if (v === undefined || v === null || v.trim() === "") return null;
+  const n = Number(v);
   return Number.isFinite(n) ? n : null;
 }
 
@@ -72,9 +75,9 @@ interface OwnershipStorageContext {
  * Builds a human-readable relationship label and officer title from the
  * reportingOwnerRelationship flags.
  */
-function describeRelationship(rel: NonNullable<
-  OwnershipDocument["reportingOwner"]
->[number]["reportingOwnerRelationship"]): { relationship: string; title: string | null } {
+function describeRelationship(
+  rel: NonNullable<OwnershipDocument["reportingOwner"]>[number]["reportingOwnerRelationship"]
+): { relationship: string; title: string | null } {
   if (!rel) return { relationship: "reporting-owner", title: null };
   const roles: string[] = [];
   if (toBool(rel.isDirector)) roles.push("director");
@@ -229,8 +232,7 @@ export async function processOwnershipForm({
   // (amendments share the base extractor), so observation rows and
   // extractor_runs/version slots never disagree. Fall back to the bare
   // document type only for forms not in the mapping.
-  const extractor_id =
-    formToExtractorId(form) ?? (str(doc.documentType) ?? form).replace("/A", "");
+  const extractor_id = formToExtractorId(form) ?? (str(doc.documentType) ?? form).replace("/A", "");
   // The XML issuerCik is authoritative. We must NOT fall back to the filing's
   // own CIK: ownership filings are ingested from a submission feed that may be
   // the reporting owner's, not the issuer's, so that fallback could stamp the
@@ -298,19 +300,27 @@ export async function processOwnershipForm({
   // Transactions: non-derivative first, then derivative, in a single index space.
   let txnIndex = 0;
   for (const t of doc.nonDerivativeTable?.nonDerivativeTransaction ?? []) {
-    await section16Repo.saveTransaction(nonDerivativeTransactionRow(t, accession_number, issuer_cik, txnIndex++));
+    await section16Repo.saveTransaction(
+      nonDerivativeTransactionRow(t, accession_number, issuer_cik, txnIndex++)
+    );
   }
   for (const t of doc.derivativeTable?.derivativeTransaction ?? []) {
-    await section16Repo.saveTransaction(derivativeTransactionRow(t, accession_number, issuer_cik, txnIndex++));
+    await section16Repo.saveTransaction(
+      derivativeTransactionRow(t, accession_number, issuer_cik, txnIndex++)
+    );
   }
 
   // Holdings: non-derivative first, then derivative.
   let holdIndex = 0;
   for (const h of doc.nonDerivativeTable?.nonDerivativeHolding ?? []) {
-    await section16Repo.saveHolding(nonDerivativeHoldingRow(h, accession_number, issuer_cik, holdIndex++));
+    await section16Repo.saveHolding(
+      nonDerivativeHoldingRow(h, accession_number, issuer_cik, holdIndex++)
+    );
   }
   for (const h of doc.derivativeTable?.derivativeHolding ?? []) {
-    await section16Repo.saveHolding(derivativeHoldingRow(h, accession_number, issuer_cik, holdIndex++));
+    await section16Repo.saveHolding(
+      derivativeHoldingRow(h, accession_number, issuer_cik, holdIndex++)
+    );
   }
 }
 

src/sec/forms/insider-trading/OwnershipDocument.test.ts

@@ -58,15 +58,18 @@ describe("OwnershipDocument parsing (Forms 3/4/5)", () => {
     expect(doc.derivativeTable?.derivativeTransaction?.length).toBe(1);
   });
 
-  it("unwraps and coerces value-wrapped leaves", async () => {
+  it("unwraps value-wrapped leaves (numerics kept as raw strings; storage coerces)", async () => {
     const xml = readFileSync(
       join(__dirname, "mock_data", "form-4", "000149315226025476-primary_doc.xml"),
       "utf-8"
     );
     const doc = await Form_4.parse("4", xml);
     const txn = doc.nonDerivativeTable!.nonDerivativeTransaction![0];
-    expect(txn.transactionAmounts?.transactionShares?.value).toBe(177936);
-    expect(txn.transactionAmounts?.transactionPricePerShare?.value).toBe(1.405);
+    // VALUE_NUMBER's inner value is typed as a string so an empty <transactionShares><value/></transactionShares>
+    // survives Value.Convert intact (as "") instead of becoming a fabricated 0. Storage's num() helper
+    // is the single place that coerces populated strings to numbers and "" to null.
+    expect(txn.transactionAmounts?.transactionShares?.value).toBe("177936");
+    expect(txn.transactionAmounts?.transactionPricePerShare?.value).toBe("1.405");
     expect(txn.securityTitle?.value).toBe("Common Stock");
   });