diff --git a/PLAN.md b/PLAN.md index 4088461..dbd32a7 100644 --- a/PLAN.md +++ b/PLAN.md @@ -226,6 +226,6 @@ The MVP is done. Nothing below blocks it; each is a known gap rather than a bug. | | Gap | |---|---| -| **NFR-1** | The performance targets have never been measured. `honojs/hono` is the fixed benchmark repository (`docs/RISKS.md` R3); the timings are not. | +| **NFR-1** | **Measured** — three of four targets pass with 20-50x of headroom; see `PRD.md` § NFR-1, measured. The fourth, 60 fps at 2,000 nodes, could not be tested at that scale: no real map on the benchmark repository gets past a few hundred nodes. | | **FR-7** | Several file cards on the canvas at once, cut deliberately in Phase 3b (`docs/UI_GUIDE.md` §6). | | **FR-9** | An incremental re-parse still re-parses the whole repository; only the write is scoped (R35). | diff --git a/PRD.md b/PRD.md index 0ad1898..52e25a8 100644 --- a/PRD.md +++ b/PRD.md @@ -76,13 +76,33 @@ database, saved canvas layouts, real-time multi-user editing. Rationale for each | | Requirement | |---|---| -| NFR-1 | **Performance** — 300-file TypeScript repo parsed and resolved in under 90 s; `functions-for-file` p95 under 150 ms; 5-hop traversal over 10k edges under 500 ms; 60 fps at 2,000 visible nodes. | +| NFR-1 | **Performance** — 300-file TypeScript repo parsed and resolved in under 90 s; `functions-for-file` p95 under 150 ms; 5-hop traversal over 10k edges under 500 ms; 60 fps at 2,000 visible nodes. **Measured** — see below. | | NFR-2 | **Security** — no network egress during parse; an untrusted repo cannot read host files; webhooks are replay- and flood-safe; every graph endpoint is session-gated; secrets come from the environment. | | NFR-3 | **Correctness** — re-parsing a renamed or deleted function leaves no orphan edges, and resolution never claims certainty it does not have. | | NFR-4 | **Operability** — one `docker compose up` brings the whole stack up; `/healthz` on api and parser; structured logs (zap in the parser, pino in the api). **Met on the clone-and-run branch**, verified from a clean clone with `make start` never run. | | NFR-5 | **Maintainability** — shared TypeScript types only in `packages/shared`; one SQL migration source at `services/parser/migrations/`; explicit SQL via sqlx in Go, Drizzle in the API, no full ORM anywhere. | | NFR-6 | **UX** — dark-mode-first with accent tokens; motion that explains rather than decorates; skeleton loading; actionable errors; `prefers-reduced-motion` respected. | +### NFR-1, measured + +Against `honojs/hono`, the benchmark repository fixed in `docs/RISKS.md` R3: 358 files, 1,472 +functions, 37,116 call sites, 5,292 stored edges. One laptop, Postgres in Docker, everything cold. + +| Target | Result | | +|---|---|---| +| 300-file TypeScript repo parsed and resolved under **90 s** | **3.2–4.5 s** over three runs, `git clone` included. **3.9 s** with the Postgres write. Peak RSS 72 MB. | pass | +| `functions-for-file` p95 under **150 ms** | **3.1 ms** — every file in the repository, three passes, n=1,065. p50 1.6 ms, max 12.8 ms. | pass | +| 5-hop traversal over 10k edges under **500 ms** | **4.4 ms** p95 over 600 samples. Worst single case **9.1 ms**, for the largest reachable set in the repository (392 nodes). | pass | +| **60 fps** at 2,000 visible nodes | Not reachable here. Opening and expanding every function in hono's largest file produces **210 nodes**, and no real map on this repository approaches the 2,000-node `NODE_CEILING`. At 210 nodes a continuous pan holds 60 fps with no frame over budget: p50 16.7 ms, p95 17.0 ms, max 17.1 ms. | not tested at scale | + +Two things worth knowing before re-running this: + +- **The parser's summary counts call sites; the database stores deduplicated edges.** 37,116 against + 5,292 is not a bug and not a discrepancy to chase. +- **The API rate-limits at 100 requests per minute**, so any latency run above that rate measures + 429s, which are fast and make the numbers look better than they are. Raise it deliberately for a + benchmark and put it back afterwards. + ## 7. Success metrics - A real 300-file public TypeScript repo parses, resolves, and renders without error or OOM. diff --git a/README.md b/README.md index 03a92c4..7a10d64 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,10 @@ GitHub webhook at it and the graph follows your pushes, rewriting only the rows **There is no hosted instance.** You run it yourself, so the database and the graphs are yours and stay on your machine. +`honojs/hono` — 358 files, 1,472 functions — clones, parses, resolves and writes to Postgres in +**under 4 seconds**. Walking the graph is single-digit milliseconds: `functions-for-file` p95 3.1 ms, +a 5-hop traversal 4.4 ms. Numbers and method in [`PRD.md`](PRD.md) § NFR-1. + ## Languages Eight, at two different depths. Support is not uniform, so it is not presented as though it were. diff --git a/docs/RISKS.md b/docs/RISKS.md index 02a47d1..4060067 100644 --- a/docs/RISKS.md +++ b/docs/RISKS.md @@ -33,7 +33,7 @@ it was. | **R30** | `/auth/dev-login` mints a session with no GitHub round trip. | **Deleted in Phase 4.** The `NODE_ENV` gate meant it only had to be wrong once. Every gated test needed it, which is what kept it alive; the test helper signs a session cookie directly through `app.signCookie` now, so the shortcut lives where it cannot be deployed. | | **R31** | A request that touches Redis hangs indefinitely when Redis is down. | **Closed in Phase 4.** Connect and command timeouts plus a finite retry ceiling, and one root error handler mapping those failures to **503** rather than 500 -- the request was fine, the dependency was not. Handled once rather than guarded per call site, because a guard per caller is how the next caller ends up without one. | | **R18** | UI direction was undefined, which is how products end up looking like scaffolding. | Decided in `docs/UI_GUIDE.md`: dark-mode-first, one signature accent, tokens in Tailwind config, Framer Motion used to explain rather than decorate. | -| **R3** | **No benchmark repo had been named**, so "a ~300-file TypeScript project" in every NFR-1 target was not testable against anything real. | **`honojs/hono`.** Public, TypeScript-first, 355 files and 1,460 functions, and dense in exactly the barrel re-exports and default imports the resolver is weakest at — it produces all three tiers with non-zero counts (1,106 / 150 / 4,650). Chosen during the 3b gate because the obvious candidate, `ARCoder181105/funcatlas` itself, is private and the parser clones anonymously. The NFR-1 *timings* are still unmeasured; the repository they will be measured against is now fixed. | +| **R3** | **No benchmark repo had been named**, so "a ~300-file TypeScript project" in every NFR-1 target was not testable against anything real. | **`honojs/hono`.** Public, TypeScript-first, 355 files and 1,460 functions, and dense in exactly the barrel re-exports and default imports the resolver is weakest at — it produces all three tiers with non-zero counts (1,106 / 150 / 4,650). Chosen during the 3b gate because the obvious candidate, `ARCoder181105/funcatlas` itself, is private and the parser clones anonymously. **Measured on the `benchmark` branch** — `PRD.md` § NFR-1: parse, resolve and write in under 4 s, query p95 in single-digit milliseconds. | | **R32** | A private or missing repository made `git clone` ask for a username. Unprompted, that blocks until `PARSE_TIMEOUT_MS` and reports a timeout for what is really "no such repository". | The clone runs with `GIT_TERMINAL_PROMPT=0` and empty `GIT_ASKPASS` / `SSH_ASKPASS`, so git fails immediately with "terminal prompts disabled". Found by the 3b gate on its first action, against the project's own private repo. | | **R33** | The registration dialog rendered five lines of the parser's zap JSON, stack trace included, running off the dialog and off the viewport. `stderrTail` promised the caller "not enough to ship a log file" and did the opposite. | The reason is read out of the last log line carrying an `error`, and its last line is the one taken -- git leads with "Cloning into '/tmp/…'" and puts the real "fatal: …" underneath. Non-JSON stderr still falls back to the tail. | @@ -67,6 +67,7 @@ Nothing outstanding -- R19 through R22 and R26 through R29 all closed; see Decid | | Risk | Notes | |---|---|---| | **R38** | **The parser sandbox is real, tested, and not used by the product.** `docs/SECURITY.md` ticked "clone/parse runs in an isolated container" and "parser has no outbound network access" from Phase 1 onward, `CLAUDE.md` said isolation "was built in Phase 1, not deferred", and the landing page told readers a repository is cloned into a sandbox with no network and no capabilities. All true of `make parser-isolated` and of nothing else: `repos/register.ts` runs the parser with `execFile(env.PARSER_BIN, ...)` from the queue worker, so on `make start` and in any composed stack it is a plain child process carrying the worker's network, filesystem and user. | Found by reading the spawn rather than the checklist. **The claim is corrected rather than the code**, because both fixes cost more than the gap: shelling out to `docker run` needs the Docker socket mounted into the worker, which grants root-equivalent host control — a worse property than the one it buys — and namespaces/seccomp/bubblewrap avoids that but is Linux-only and a project of its own. What survives is enforced everywhere, because it lives *in* the parser rather than around it: symlinks hard-fail, size/count/depth caps, a `--depth 1` clone with credential prompts disabled, no repo scripts executed, and the clone removed on the failure path too. Two lessons worth more than the fix. A checkbox ticked against a harness says nothing about the product — the harness passes `--no-deps`, which is also why nobody hit the `parser` service's `network_mode: none` / `depends_on` contradiction. And the deferral in the TOCTOU checklist item had quietly leaned on this claim inside its own justification, so one false statement had already propagated into a second decision. | +| **R40** | **The API rate-limits at 100 requests a minute, and a restored canvas can spend that in one load.** `@fastify/rate-limit` is registered globally with `max: 100`. Every expanded branch is its own `/edges` request, so rehydrating a large saved map fires them all at once. | Found while benchmarking, where the first run silently measured 429s -- fast, and indistinguishable from a fast API in the numbers. Not raised: 100/min is right for an instance anyone can reach, and normal exploration is a handful of requests. The failure is legible when it happens (the canvas shows an error rather than a wrong graph), and the honest fix is batching the traversal into one request rather than loosening the limit. Anyone re-running the NFR-1 numbers has to raise it deliberately and put it back -- recorded in `PRD.md` § NFR-1. | | **R39** | **`FUNCATLAS_SINGLE_USER` makes the instance unauthenticated, and nothing in the process can stop it being exposed.** Set, the API registers no OAuth routes and resolves every request to that user. `make setup` writes it by default, so the common case is a stack with no authentication at all. | **Accepted, and the mitigation is weaker than it first looked.** The plan said the server would refuse to start unless bound to loopback; it binds `0.0.0.0` and must, or Docker's port mapping cannot reach it, and a process cannot know what sits in front of it. So the protection is arrangement, not code: `make setup` writes the value into the user's own `.env` and prints what it did, compose publishes every port on `127.0.0.1` including Postgres and Redis, and the API logs a warning naming this risk on every start. Deliberately not R30's shape -- there is no endpoint to reach and no gate to be wrong, only an instance a human configured as open. Blank the value in `.env` to get real GitHub sign-in back. Revisit if this is ever deployed anywhere but a laptop. | ---