# PRD: SDK Size Reduction

**Feature Name:** sdk-size-reduction
**Branch:** fester/sdk-size-reduction
**Base Branch:** main
**Base Branch Note:** The repository has both `main` and `master` branches. `main` is the intentional target — the feature branch `fester/sdk-size-reduction` targets `main`, which then merges to `master`. This is confirmed intentional.
**Created:** 2026-03-16
**Spec:** planning/sdk-size-reduction/spec.md
**Review:** planning/sdk-size-reduction/spec-review.md
**Status:** Draft — Rev 2 (Thing Rev 1 fixes)

---

## Overview

The `avo-inspector` npm package accumulated significant dead weight: test/mock artifacts leaking into `dist/`, and always-loaded heavy dependencies (`@noble/curves`, `safe-regex2`) that are never used in production. The spec phase (Revisions 1–5) completed the majority of the work: `tsconfig.json` exclusions are correct, `package.json` `files` whitelist is correct, the ESM dual build was added, the clean build was verified at 37.6 kB packed, and the prod-env test was written.

**Note on `dist-native/`:** The `dist-native/` directory was removed from git tracking in July 2020 (commit `350b785`) and has been in `.gitignore` since then. It contains zero tracked files. `git rm -r dist-native/` will fail because there is nothing to remove from git. The directory exists only as an untracked local artifact with no impact on the repository, build, tests, or npm package — no story is required to address it.

**What remains before the feature is merge-ready:**

1. Add the ordering constraint comment to `initEventSpecModules()`.
2. Write the dynamic import failure test and success-path assertion for `initEventSpecModules()`.
3. Run the full `prepublishOnly` flow (includes example project tests).

The `.npmignore` file was already deleted from the repository root (confirmed: file not found). That acceptance criterion is satisfied.

---

## Current State (Verified by Morticia Rev 5)

| Item | State |
|------|-------|
| `tsconfig.json` exclude array | Correct: `["src/__tests__", "src/__mocks__", "src/browser.js", "src/script.js"]` |
| `package.json` files whitelist | Correct: whitelist with exclusions for `__tests__`, `__mocks__`, `browser.js`, `script.js`, `*.LICENSE.txt` |
| `package.json` sideEffects | `"sideEffects": false` present |
| `package.json` ESM exports | `"module"` field + `"exports"` conditional present; `tsconfig.esm.json` present; `dist/esm/` built |
| `src/index.ts` AvoStreamId export | Present at line 3 |
| `AvoInspector.ts` dynamic imports | `initEventSpecModules()` uses `Promise.all([import(...)])` pattern |
| `AvoSchemaParser.ts` lazy encryption | Module-level `_encryptValueFn` cache + `await import()` pattern |
| `AvoInspector.ts` line 141 comment | Corrected |
| Clean build verification | `rm -rf dist && tsc && tsc -p tsconfig.esm.json` exits 0; no stale artifacts |
| Packed tarball size | 37.6 kB packed / 41 files / 160.8 kB unpacked (≤ 38 kB criterion met) |
| Prod-env test | Present in `AvoInspectorEventSpec_test.ts` lines 345–354 |
| `.npmignore` | Deleted (file not found at repo root) |
| `dist-native/` | Gitignored and untracked since 2020 (commit `350b785`); zero tracked files; no action required |
| Dynamic import failure test | NOT WRITTEN |
| Dynamic import success-path test | NOT WRITTEN |
| Ordering constraint comment | NOT ADDED |
| Full prepublishOnly flow | NOT RUN |

---

## Stories

### Story 1: Add ordering constraint comment to `initEventSpecModules()`

**Priority:** medium
**Depends on:** none
**Estimated files changed:** 1 (`src/AvoInspector.ts`)

**Description:**
The `initEventSpecModules()` method in `src/AvoInspector.ts` (starting at line 147) has a subtle ordering constraint: all instance field assignments (`this.eventSpecCache`, `this.eventSpecFetcher`, `this._validateEvent`) must occur inside the `try` block, before the `finally` block sets `this._eventSpecReady = undefined`. Concurrent callers that `await this._eventSpecReady` will resume after the promise resolves and must find all fields already populated. If assignments were moved after or into the `finally` block, a race condition would be introduced where concurrent callers see undefined fields immediately after the promise resolves.

This constraint is documented only in the spec edge cases section. It must be documented in the code itself (Remaining Work #13).

Add a multi-line comment directly above the `try` block inside `initEventSpecModules()` (before line 148) explaining the ordering requirement. The comment must explicitly mention `_eventSpecReady`, `finally`, and the race condition risk.

**Acceptance Criteria:**
- `src/AvoInspector.ts` `initEventSpecModules()` contains a comment above the `try` block documenting the ordering constraint.
- The comment explicitly mentions `_eventSpecReady`, `finally`, and the race condition risk for concurrent callers.
- The comment explains that assignments must precede the `finally` block so concurrent `await _eventSpecReady` callers find all fields populated.
- `yarn build` exits 0.
- `yarn test --roots='<rootDir>/src'` passes.
- `BROWSER=1 yarn test --roots='<rootDir>/src'` passes.

**Quality Checks:**
- `yarn build` (required)
- `yarn test --roots='<rootDir>/src'` (required)
- `BROWSER=1 yarn test --roots='<rootDir>/src'` (required)

**Test Pattern:** No new tests. Comment-only change to `src/AvoInspector.ts`; existing suite must pass unchanged.

---

### Story 2: Add dynamic import failure test and success-path assertion for `initEventSpecModules()`

**Priority:** high
**Depends on:** none (independent of Story 1)
**Estimated files changed:** 1 (`src/__tests__/AvoInspectorDynamicImportFailure_test.ts` — new file)

**Description:**
The `initEventSpecModules()` method in `src/AvoInspector.ts` has a `try/catch` block that handles dynamic import failures. The graceful degradation path — where a failed import causes `trackSchemaFromEvent` to fall through to the batched flow without surfacing an error to the caller — has no test coverage (Remaining Work #12). Additionally, the spec acceptance criterion requires asserting that in a dev/staging environment with a valid `streamId`, `initEventSpecModules()` successfully loads all three eventSpec modules and `ensureEventSpecReady()` returns a non-null object with `cache`, `fetcher`, `validate`, and `streamId` fields.

**CRITICAL — Test isolation requirement:**

Do NOT add these tests to the existing `src/__tests__/AvoInspectorEventSpec_test.ts`. That file has a top-level import of `AvoInspector` (line 1: `import { AvoInspector } from "../AvoInspector"`), which is resolved by Jest before any `jest.doMock()` call inside a test body. To intercept the dynamic `require()` calls compiled from `import()` expressions (TypeScript compiles `import()` to `Promise.resolve().then(() => require(...))` under `module: CommonJS`), the test must:

1. Call `jest.resetModules()` to clear the module registry
2. Call `jest.doMock("../eventSpec/AvoEventSpecCache", ...)` (and similarly for the other two eventSpec modules) to register mock factories
3. Re-require `AvoInspector` **inside the test body** using `const { AvoInspector } = require("../AvoInspector")` after mocking

Create a new, isolated test file: **`src/__tests__/AvoInspectorDynamicImportFailure_test.ts`**

This file must NOT have a top-level `import` of `AvoInspector`. Instead, require `AvoInspector` inside each test body after `jest.resetModules()` + `jest.doMock()`.

**Test 1 — Failure path (graceful degradation):**
```typescript
test("should degrade gracefully when dynamic imports fail", async () => {
  jest.resetModules();
  jest.doMock("../eventSpec/AvoEventSpecCache", () => { throw new Error("import failed"); });
  jest.doMock("../eventSpec/AvoEventSpecFetcher", () => { throw new Error("import failed"); });
  jest.doMock("../eventSpec/EventValidator", () => { throw new Error("import failed"); });

  const { AvoInspector } = require("../AvoInspector");
  const { AvoInspectorEnv } = require("../AvoInspectorEnv");

  const inspector = new AvoInspector({ apiKey: "test-key", env: AvoInspectorEnv.Dev, version: "1.0.0" });

  // Wait for async init to settle (it will catch the error)
  if ((inspector as any)._eventSpecReady) {
    await (inspector as any)._eventSpecReady.catch(() => {});
  }
  await new Promise(resolve => setTimeout(resolve, 50));

  // Assert: fields are undefined (failure path)
  expect((inspector as any).eventSpecCache).toBeUndefined();

  // Assert: tracking still works (batched flow)
  const result = await inspector.trackSchemaFromEvent("test_event", { prop: "value" });
  expect(result).toBeDefined();
  expect(result.length).toBeGreaterThan(0);

  jest.resetModules();
});
```

**Test 2 — Success path:**
```typescript
test("should load all three eventSpec modules successfully in dev env", async () => {
  jest.resetModules();
  // No mocks — use real modules

  const { AvoInspector } = require("../AvoInspector");
  const { AvoInspectorEnv } = require("../AvoInspectorEnv");

  const inspector = new AvoInspector({ apiKey: "test-key", env: AvoInspectorEnv.Dev, version: "1.0.0" });

  if ((inspector as any)._eventSpecReady) {
    await (inspector as any)._eventSpecReady;
  }

  const result = await (inspector as any).ensureEventSpecReady();
  // In a test environment streamId may be "unknown" — result may be null if streamId guard fires.
  // Assert module loading completed (eventSpecCache populated after successful import).
  expect((inspector as any).eventSpecCache).toBeDefined();
  expect((inspector as any).eventSpecFetcher).toBeDefined();
  expect((inspector as any)._validateEvent).toBeDefined();

  jest.resetModules();
});
```

**Note:** If `ensureEventSpecReady()` returns `null` due to the `streamId === "unknown"` edge case in tests, the test should assert on the individual loaded fields (`eventSpecCache`, `eventSpecFetcher`, `_validateEvent`) rather than on the return value of `ensureEventSpecReady()`. The spec criterion is that the modules load successfully — the individual field assertions verify this.

**Acceptance Criteria:**
- A new test file `src/__tests__/AvoInspectorDynamicImportFailure_test.ts` is created.
- The file does NOT have a top-level `import` of `AvoInspector` — `AvoInspector` is required inside each test body after `jest.resetModules()`.
- The failure-path test asserts `trackSchemaFromEvent` returns a **defined array with length > 0** (not just "resolves") when `initEventSpecModules()` fails due to a mocked import error.
- The failure-path test asserts `(inspector as any).eventSpecCache` is `undefined` after the failure.
- The success-path test asserts `(inspector as any).eventSpecCache`, `(inspector as any).eventSpecFetcher`, and `(inspector as any)._validateEvent` are all defined after successful module loading.
- `yarn test --roots='<rootDir>/src'` passes including both new tests.
- `BROWSER=1 yarn test --roots='<rootDir>/src'` passes.
- `yarn build` exits 0.

**Quality Checks:**
- `yarn build` (required)
- `yarn test --roots='<rootDir>/src'` (required)
- `BROWSER=1 yarn test --roots='<rootDir>/src'` (required)

**Test Pattern:** New isolated test file `src/__tests__/AvoInspectorDynamicImportFailure_test.ts`. No top-level `AvoInspector` import — use `jest.resetModules()` + `jest.doMock()` + `require()` inside each test body. Two tests: (1) failure path — mock all three eventSpec modules to throw, assert `trackSchemaFromEvent` returns a defined array with length > 0 and `eventSpecCache` is `undefined`; (2) success path — assert `eventSpecCache`, `eventSpecFetcher`, `_validateEvent` are defined after `_eventSpecReady` settles.

---

### Story 3: Run and verify full `prepublishOnly` flow

**Priority:** high
**Depends on:** Story 1 (comment added), Story 2 (tests written)
**Estimated files changed:** 0 (verification only; up to 2 if ESM dual build causes regressions)

**Description:**
The full publish gate (`prepublishOnly`) is:
```
yarn build && yarn test:browser && rm -rf examples/ts-avo-inspector-example/node_modules/ && cd examples/ts-avo-inspector-example/ && yarn && yarn test --watchAll=false
```

This must be run in full and must pass before the feature is considered merge-ready. It exercises:
- Clean build (`rm -rf dist && tsc && tsc -p tsconfig.esm.json`)
- Browser test suite (`BROWSER=1 jest`)
- Example project: fresh install from the local package, then all example tests

The example project (`examples/ts-avo-inspector-example/`) uses `"avo-inspector": "file:../.."` so it installs from the local source. Its test suite uses `react-scripts test` (Jest + React Testing Library).

If any test fails in the example project due to the ESM dual build (`"module"` + `"exports"` fields in `package.json`), those failures must be diagnosed and fixed in this story.

**Note:** `dist-native/` is gitignored and untracked (zero git-tracked files since commit `350b785`, July 2020). It has no impact on this flow and requires no cleanup action.

**Acceptance Criteria:**
- `yarn prepublishOnly` (or the equivalent manual sequence) exits 0.
- All phases pass: `yarn build`, `yarn test:browser`, example project `yarn test --watchAll=false`.
- No regressions introduced by the ESM dual build (`"module"`, `"exports"` fields) in the example project.
- The acceptance criterion `[ ] Example project tests pass` in the spec is satisfied.

**Quality Checks:**
- `yarn build` (required)
- `yarn prepublishOnly` (required)
- `cd examples/ts-avo-inspector-example && yarn test --watchAll=false` (required)

**Test Pattern:** No new unit tests. Verification only. Run `yarn prepublishOnly` end-to-end. Fix any example project test failures caused by ESM dual build `package.json` fields if found.

---

## Dependency Graph

```
Story 1 (ordering comment)    ──────┐
                                    ├──► Story 3 (prepublishOnly)
Story 2 (import tests)        ──────┘
```

Stories 1 and 2 are independent and can be implemented in parallel. Story 3 depends on both being complete.

---

## Risk Assessment

| Risk | Likelihood | Impact | Mitigation |
|------|-----------|--------|-----------|
| `jest.doMock` + `jest.resetModules` + `require()` pattern does not intercept the CJS `require()` compiled from `import()` | Low | Story 2 failure path test is harder to write; may need alternative approach | Pre-specified: use new isolated test file `AvoInspectorDynamicImportFailure_test.ts` with no top-level AvoInspector import; require AvoInspector inside test body after mocking |
| `streamId === "unknown"` in test environment causes `ensureEventSpecReady()` to return null | Medium | Success-path test may need to assert on `eventSpecCache`/`eventSpecFetcher`/`_validateEvent` fields directly rather than on `ensureEventSpecReady()` return | Test pattern pre-specifies asserting individual fields as fallback; both assertions verify module loading |
| ESM dual build (`"exports"` field) breaks example project's `react-scripts` module resolution | Low | Story 3 fails; need to adjust `package.json` exports | The `"browser"` field fallback remains in `package.json`; `react-scripts` uses webpack which respects `"browser"` |
| `yarn build` after comment addition produces unexpected output | Very Low | None expected | Comment-only change has zero impact on TypeScript compilation output |

---

## File Reference

| File | Story | Change Type |
|------|-------|-------------|
| `src/AvoInspector.ts` | Story 1 | Add comment to `initEventSpecModules()` above `try` block (around line 147) |
| `src/__tests__/AvoInspectorDynamicImportFailure_test.ts` | Story 2 | New file — isolated test with failure-path and success-path tests |
| `examples/ts-avo-inspector-example/` | Story 3 | Verify/fix if needed |

---

## Already Completed (Do Not Re-Implement)

The following were completed during the spec phase (Revisions 1–5) and are verified correct in the current codebase:

- `tsconfig.json`: correct `exclude` array, no stale entries, `target: ES2020`
- `tsconfig.esm.json`: present, extends `tsconfig.json`, outputs to `dist/esm/` with `module: ESNext`
- `package.json`: `"files"` whitelist, `"sideEffects": false`, `"module"` field, `"exports"` conditional, `build` script runs `tsc && tsc -p tsconfig.esm.json`
- `src/index.ts`: exports `AvoStreamId`
- `src/AvoInspector.ts`: `initEventSpecModules()` uses `Promise.all([import(...)])`, line 141 comment corrected
- `src/AvoSchemaParser.ts`: lazy `_encryptValueFn` cache pattern
- `src/__tests__/AvoInspectorEventSpec_test.ts`: prod-env test at lines 345–354
- `.npmignore`: deleted from repository root
- `dist-native/`: already gitignored and untracked since 2020 — no story needed
- Clean build: verified at 37.6 kB packed / 41 files / 160.8 kB unpacked

---

## Revision History

| Rev | Date | Author | Summary |
|-----|------|--------|---------|
| 1 | 2026-03-16 | Pugsley | Initial PRD: 4 stories (delete dist-native/, ordering comment, dynamic import failure test, prepublishOnly verification) |
| 2 | 2026-03-17 | Pugsley | Thing Rev 1 fixes: deleted Story 1 (dist-native/ is gitignored/untracked since 2020 — git rm will fail; no action needed); clarified baseBranch: main is intentional; changed Story 3 approach to test-driven; pre-specified Story 3 mock pattern as new isolated file AvoInspectorDynamicImportFailure_test.ts with jest.resetModules + jest.doMock + require() pattern; added success-path test to cover spec AC for dev/staging valid streamId; set Story 4 estimatedFiles to 0 with contingency note; tightened Story 2 AC to require specific terms (_eventSpecReady, finally, race condition); tightened Story 3 "resolves" criterion to specify defined array length > 0; replaced yarn test in Story 4 quality checks with yarn prepublishOnly; increased maxIterations from 8 to 12; renumbered stories 1–3 after Story 1 deletion |