Add regression test suites, fix backend TS build, reorganize dev docs

[NimeshaKahingala]

Backend was unrunnable locally: TypeScript was pinned to ^4.0.0 while @azure/* and pg's installed type definitions require TS 5+. Bumped typescript to ^5.7.3 and @types/node to ^22.x to match.

Added a regression safety net ahead of the codebase cleanup effort:

• packages/back-end: vitest unit tests for pure helpers + black-box API tests against the real running func host (20 tests)
• packages/e2e: Playwright suite driving the actual main workflow (home -> product detail -> related items -> supply-tree match) (5 tests)

These surfaced two real pre-existing bugs, captured as regression baselines rather than fixed here: GET /getRelatedOKH always resolves to an empty list (no {keywords} route param), and supplyTree.vue's heading never renders (non-reactive var instead of a ref). Also excluded test/ from the backend's tsc build (it was breaking npm run build).

Moved ARCHITECTURE_ANALYSIS.md, CONTAINERIZATION_CHECKLIST.md, and the two new planning docs into dev-docs/ to keep the repo root clean, and added AGENT.md at the root as an orientation doc for coding agents.

[Copilot]

🟡 Changes recommended

API and E2E tests need greater determinism and timeout isolation, and several documentation counts and references are inconsistent.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds backend and E2E regression coverage, updates TypeScript build tooling, and reorganizes development documentation.

Changes:

• Adds Vitest unit/API tests and Playwright workflow tests.
• Updates TypeScript dependencies and excludes tests from production builds.
• Moves development documentation and adds agent guidance.

File summaries

File	Summary
packages/e2e/tests/main-workflow.spec.ts	Main browser workflow tests
packages/e2e/tests/fixtures.ts	Shared page-error fixture
packages/e2e/playwright.config.ts	Playwright and server configuration
packages/e2e/package.json	E2E scripts and dependency
packages/back-end/vitest.config.mts	Vitest configuration
packages/back-end/tsconfig.json	Excludes tests from production builds
packages/back-end/test/unit.test.ts	Backend unit tests
packages/back-end/test/setup.ts	Test environment setup
packages/back-end/test/api.test.ts	Black-box API regression tests
packages/back-end/src/functions/httpFunctions.ts	Exports tested helpers
packages/back-end/package.json	Tooling and test scripts
dev-docs/TESTING.md	Testing documentation
dev-docs/CONTAINERIZATION_CHECKLIST.md	Containerization checklist
dev-docs/CLEANUP_PLAN.md	Cleanup roadmap
dev-docs/ARCHITECTURE_ANALYSIS.md	Architecture documentation
AGENT.md	Repository orientation guidance
.gitignore	Playwright artifact exclusions

Review details

Suppressed comments (8)

AGENT.md:44

• This convention contradicts the file being added: AGENT.md is itself a root-level Markdown document outside README.md/LICENSE. State the intentional AGENT.md exception so future agents do not move or reject this orientation file while applying the documented layout rule.

- Root-level `*.md` docs beyond `README.md`/`LICENSE` live in `dev-docs/`, not the repo root.

AGENT.md:26

• This command comment says npm test runs 13 unit tests, but the added unit suite has 12 it cases; together with the 7 API cases the total is 19. Please keep this orientation count consistent with the actual test files.

   cd packages/back-end && npm test        # 13 unit + 7 API tests

dev-docs/CLEANUP_PLAN.md:47

• The documented test count is incorrect: unit.test.ts contains 12 it cases (3 in each of four groups) and api.test.ts contains 7, so Vitest runs 19 tests, not 20. Please update this count and the matching agent instructions so the regression baseline is accurately described.

  - Back end (`packages/back-end`, vitest): unit tests for `getFileNameAndFileType`, `hasOverlapKeywords`, `normalizeKeywords`, `convertToProduct`; black-box API tests against the real running func host covering `/test`, `/listRoutes`, `/listOKHsummaries`, `/listOKWsummaries`, `/getFile`, `/getRelatedOKH`, `/incidents`. `npm test` → 20/20 passing.

dev-docs/CLEANUP_PLAN.md:5

• The opening context still says the repository has “no automated tests”, but this PR adds the backend/API and Playwright suites and marks them complete in Phase 0 below. That present-tense statement now contradicts the plan's own status; remove the stale claim so agents do not conclude that the new test suites are absent.

The repo (Nuxt 3 front end + Azure Functions/TypeScript back end, Postgres + Azure Blob Storage) has grown organically with no CI, no automated tests, no lint/format enforcement, and no lockfiles for any package except `atoms`. A prior audit (`ARCHITECTURE_ANALYSIS.md`) and direct code review confirmed real problems in both layers, listed below. This document is the durable, versioned record of scope, phases, and status — check off phases as they land.

packages/back-end/test/api.test.ts:84

• test:api is described as a black-box regression suite for any locally running host, but this case reaches into one mutable shared Azure Blob by hard-coding a filename and title. A valid local settings/storage account without that exact object will fail even when /getFile works. Select a file discovered from the list endpoint or use a deterministic fixture/mock instead of asserting production data.

      `${BASE_URL}/getFile/okh/okh-chococolate-chip-cookies-recipe/json`
    );

packages/back-end/test/api.test.ts:42

• Only the reachability probe and the /incidents case use fetchWithTimeout; the other endpoint calls below use unbounded fetch. If /test responds but a blob-backed handler stalls, npm run test:api can hang indefinitely instead of failing fast. Route every black-box request through the existing timeout helper or apply an equivalent per-test timeout.

    const res = await fetch(`${BASE_URL}/listRoutes`);

packages/back-end/test/api.test.ts:106

• This test does not actually verify the behavior stated in its name/comment: it probes only keywords=cookies, and the expected empty array depends on mutable Azure blob contents. A route that still ignores queries could start returning a match for a different query without failing this test, while an unrelated blob tagged undefined would fail it. Exercise multiple distinct query strings against deterministic fixture data, or compare query results without hard-coding the live storage contents.

  it("currently always returns an empty relatedOKH list, regardless of the query string", async () => {
    const res = await fetch(`${BASE_URL}/getRelatedOKH?keywords=cookies`);
    expect(res.status).toBe(200);
    expect(await res.json()).toEqual({ relatedOKH: [] });

packages/e2e/tests/main-workflow.spec.ts:81

• sendToSupplyGraphAI is started from onMounted, but this assertion runs before that async flow has settled, while the only wait is after it. The config also does not control OHM: if localhost:8001 is running and returns solutions, .supply-tree will appear and this test fails for an unrelated environment. Intercept **/v1/api/match (or otherwise force an offline response), await the request, and then assert the final heading/tree state.

  await expect(page.locator("h1")).toHaveText("");

• Files reviewed: 14/17 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The added API and Playwright suites are not included in CI, and test/documentation accuracy issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• packages/e2e/package-lock.json: Generated file

Suppressed comments (8)

AGENT.md:39

• This says there is no CI, but this PR adds .github/workflows/ci.yml and it already runs the back-end build/unit tests and front-end build. Update the sharp-edge note to distinguish the CI that exists from the API/Playwright suites that are still not wired into it.

- **No CI, no lint config, no `.env.example` files yet.** These are `CLEANUP_PLAN.md` Phase 0/2 items, not done — don't assume a red flag you'd normally expect CI to have caught actually got caught.

AGENT.md:26

• The current unit suite contains 12 tests, not 13, so this command note overstates the total (npm test runs 19 tests including the 7 API cases). Please update the count to avoid sending agents looking for a nonexistent test.

   cd packages/back-end && npm test        # 13 unit + 7 API tests

dev-docs/CLEANUP_PLAN.md:36

• This finding is stale for three packages: this PR commits lockfiles for packages/back-end, packages/front-end, and packages/e2e; only packages/mock-api remains ignored. As written, the cleanup plan misstates which installs are now reproducible.

- `package-lock.json` gitignored for `back-end`, `front-end`, `mock-api` (only `atoms` has one committed) — installs aren't reproducible.

dev-docs/CLEANUP_PLAN.md:47

• The added unit suite has 12 it cases (three in each of the four groups), plus 7 API cases, so the documented total is 19/19 rather than 20/20. Please correct this baseline count so the cleanup plan accurately reflects what the regression gate covers.

  - Back end (`packages/back-end`, vitest): unit tests for `getFileNameAndFileType`, `hasOverlapKeywords`, `normalizeKeywords`, `convertToProduct`; black-box API tests against the real running func host covering `/test`, `/listRoutes`, `/listOKHsummaries`, `/listOKWsummaries`, `/getFile`, `/getRelatedOKH`, `/incidents`. `npm test` → 20/20 passing.

dev-docs/CLEANUP_PLAN.md:36

• This status text is stale in the same PR: .github/workflows/ci.yml now exists, and lockfiles for the back end/e2e package are added (the mock-api lockfile is still pending). Leaving the findings as “no workflows” and “only atoms has one” will mislead agents about the current repository state.

- No `.github/workflows` — nothing mechanically catches a regression today.
- `package-lock.json` gitignored for `back-end`, `front-end`, `mock-api` (only `atoms` has one committed) — installs aren't reproducible.

packages/back-end/test/api.test.ts:129

• These two hard-coded queries are not guaranteed to produce different match sets. If the live storage has no cookies keyword (or neither query matches), a correctly query-aware endpoint can return the same empty array and this test will still pass, so it does not actually prove that the query is ignored. Discover a real keyword from a fetched manifest (or use a fixture) and compare it with a guaranteed non-matching sentinel before asserting the current buggy equality.

  it("ignores the keywords query param (two different queries return the same result)", async () => {
    const [resA, resB] = await Promise.all([
      fetchWithTimeout(`${BASE_URL}/getRelatedOKH?keywords=cookies`),
      fetchWithTimeout(`${BASE_URL}/getRelatedOKH?keywords=something-entirely-different`),
    ]);

packages/e2e/tests/main-workflow.spec.ts:85

• page.waitForRequest resolves when the request is created, not after the intercepted request fails or the fetch rejection reaches catch/finally. The assertions after this await can therefore pass while the failure is still in flight, so an unhandled rejection or later rendering error is not covered; wait for the request-failed/settled UI state before asserting graceful degradation.

  await matchRequest;

packages/e2e/tests/main-workflow.spec.ts:80

• This assertion requests a 60-second expectation timeout, but the config leaves Playwright's per-test timeout at its 30-second default. If the slow navigation described in the nearby comments takes over 30 seconds, the test is terminated before this timeout can provide the advertised safety margin; increase the test timeout as well.

  await expect(page).toHaveURL(/\/products\/.+\/supplyTree/, { timeout: 60000 });

• Files reviewed: 15/21 changed files
• Comments generated: 5
• Review effort level: Lite

[NimeshaKahingala]

This is deliberate, not an oversight — the user explicitly asked to keep CI minimal (no live blob storage dependency, no browser download). Both suites need one or the other (test:api needs a live backend against real Azure blob data; Playwright needs a ~275MB browser). Added a header comment to ci.yml making this scope explicit, and dev-docs/TESTING.md documents it. They remain local-only pre-merge checks, re-run manually before each cleanup-plan phase.

[NimeshaKahingala]

Fixed in 2e59740 — reworded to reflect that the lockfile is now committed and CI now exists (build+unit only), rather than describing the pre-#106 state as current.

[NimeshaKahingala]

Fixed in 2e59740 — updated to reflect that lockfiles exist for back-end/front-end/e2e (only mock-api remains) and CI now exists (build+unit only, not the full regression gate).

[NimeshaKahingala]

Fixed in 2e59740 — replaced with an accurate description: ci.yml exists and runs build+unit tests, but deliberately not test:api or the Playwright suite (kept out for the reasons noted in the ci.yml comment and this file).

[NimeshaKahingala]

Fixed in 2e59740 — the test now actually visits both the home page and a product detail page (discovered via the first product card's href) rather than only checking "/" while claiming to check every page visited earlier in the suite.

[NimeshaKahingala]

Responding to the rest of the second review pass (these were in the collapsed "Suppressed comments" section, not individual threads, so replying here instead):

packages/back-end/test/api.test.ts — getRelatedOKH test. This led to a real, important correction, not just a test-design fix. Investigating the "discover a real keyword vs sentinel" suggestion, I found the test's entire premise was wrong: GET /getRelatedOKH does not ignore the keywords query param. I'd reasoned "no {keywords} route template → request.params.keywords must be undefined" — that doesn't hold for this Azure Functions host, whose RPC binding data flattens query-string values into request.params even without a matching route segment. Empirically verified:

?keywords=cookie%20-%20chocolate  → matches the file tagged exactly that
?keywords=COOKIE%20-%20CHOCOLATE  → matches (case-insensitive, as designed)
?keywords=cookies                 → no match (correctly — no file has that literal token)

The endpoint's keyword matching actually works. Fixed in 2e59740: the test now discovers a real file's keyword and asserts a matching query returns it while an unrelated one doesn't — a genuine positive test of working behavior, not a bug-documentation test. Retracted the claim everywhere else it appeared (dev-docs/CLEANUP_PLAN.md, TESTING.md, AGENT.md) and closed/corrected issue #107.

packages/e2e/tests/main-workflow.spec.ts:85 (await matchRequest). Valid — page.waitForRequest resolves when the request is created, not when it settles. Fixed in 2e59740: swapped for page.waitForEvent("requestfailed", ...), which resolves when the aborted request actually fails.

packages/e2e/tests/main-workflow.spec.ts:80 (60s timeout vs 30s test default). Confirmed and fixed — this was a real bug, and explains the intermittent full-suite failure I'd seen earlier: Playwright's default 30s test-level timeout was killing the test before the 60s assertion timeout could ever matter. Added test.setTimeout(90_000) in 2e59740.

Test count ("12 unit tests, not 13" / "19 not 20 total"). Checked, not changed — cd packages/back-end && npx vitest run test/unit.test.ts --reporter=verbose shows 13 tests passing (13), not 12. The original 20/20 total (13 unit + 7 API) in the docs was already correct.

All pushed to this branch (2e59740). Backend suite: 20/20 passing. E2E suite: 5/5 passing (stable across repeated runs; documented one occasional slow-navigation flake under full-sequential-suite load in this dev sandbox in dev-docs/TESTING.md, not reproducible in isolation).