From ce700c8de0d4cac4ce25cd933f81d0ed2717bee8 Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sun, 30 Aug 2026 10:37:24 +0530 Subject: [PATCH] measure NFR-1 against hono Three of the four targets pass with 20-50x of headroom: 358 files cloned, parsed, resolved and written in 3.9s against a 90s budget; functions-for-file p95 3.1ms against 150ms; a 5-hop traversal 4.4ms p95, 9.1ms worst case, against 500ms. The fourth could not be tested at its stated scale. Opening and expanding every function in hono's largest file produces 210 nodes, and nothing on this repository approaches the 2,000-node ceiling. What was measured at 210 nodes is recorded rather than extrapolated. Two traps found by running it, both recorded so the next person does not lose an hour to them. The parser's summary counts call sites and the database stores deduplicated edges, so 37,116 against 5,292 is not a discrepancy. And the API rate-limits at 100 requests a minute, which the first latency run silently measured instead of the API -- 429s are fast. That second one is R40: it also means a restored canvas with many open branches can spend the budget on one load. --- PLAN.md | 2 +- PRD.md | 22 +++++++++++++++++++++- README.md | 4 ++++ docs/RISKS.md | 3 ++- 4 files changed, 28 insertions(+), 3 deletions(-) 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. | ---