Repository navigation
Add a codebase map to docs/ - #1646
dev-aditya-hub wants to merge 1 commit into
Conversation
Record a structural survey of the repository as it stands on main: the package layout and dependency graph, the startup/shutdown and background-worker lifecycle, the static-import and GTFS-RT data flows with their unit and timezone conventions, the full route table with middleware ordering, the configuration surface and its consumers, and the test and CI topology. References are given as file:line so the document can be checked against the tree rather than trusted. The motivation is onboarding cost. Behaviour here is concentrated in a few large shared engines (BuildTripStatus, arrivals_core, scheduled_block_helper, trip_updates_helper, reference_utils) that every real-time endpoint routes through, and the Java-parity reasoning for much of it lives only in code comments. Having the map written down means that context does not have to be rebuilt from scratch each time. Writing it also surfaced eleven places where the contributor guides describe behaviour the code does not implement: rate limiting is one shared bucket rather than per-key, CONTRIBUTING.md names two helper files that do not exist, CLAUDE.md still documents the gtfsData in-memory structures that were removed, and the README documents a HEALTH_CHECK_KEY variable nothing reads. Those are recorded here as neutral observations with evidence; they are raised separately in OneBusAway#1644 and fixed in OneBusAway#1645 rather than in this document. Section M states coverage honestly: every tracked file has a status, every non-test source file was opened, and roughly 85% of non-test source lines were read line by line, with the unread remainder enumerated. The full test suite was not run - this machine has no C compiler, so the default CGO mode is unavailable; go vet and a subset of packages pass under -tags purego, which is noted as not establishing CGO-specific coverage. Per docs/README.md the wiki remains canonical for specifications. This is a point-in-time artifact rather than a spec, so it has no wiki mirror and docs/AGENTS.md needs no new row.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes: 1
⚙️ Run configuration
📒 Files selected for processing: 1
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
aaronbrethorst
left a comment
aaronbrethorst
left a comment
There was a problem hiding this comment.
Thanks for putting this together, @dev-aditya-hub. It's obvious you put real care into it, and #1644/#1645 are useful things to have come out of it. I don't want to merge it in its current form, though. Here's why:
It's already out of date on main. The branch sits on d07c60a, and main has moved since then. For example:
- About 55 of the ~240
file:linereferences now point at the wrong line, among themBuildTripStatus(trips_helper.go:50, now :51) andpollFeed(realtime.go:981, now :1006). - D.4 says arrivals are sorted by scheduled arrival. Since #1576 they're sorted by best arrival time (predicted, else scheduled) after
buildArrival. - C.4 and H.4 describe
docker-publish.ymland GHCR. Both were removed in #1609. - The H.1 version pins are stale too.
A doc made of line numbers goes stale with every merge, and nobody is going to keep 220 of them current.
Some claims were wrong even at d07c60a:
- The doc says "27 registered routes" in several places, and K.4 #3 builds on that number.
SetRoutesregisters 34/apiroutes, and your own F.1 table lists 34. - H.1 says
golang.org/x/synchas no non-test import.advanced_direction_calculator.go:15importssingleflight. - E.3 says there are 17 base tables.
schema.sqlcreates 16.
Where it lives. docs/ is for historical artifacts, and the wiki is canonical. Sections A.1, I.4, L and M describe your machine (/mnt/c/Users/..., /home/...) and your process, not the project.
If you'd like to keep going, I'd take a much shorter "Architecture overview" wiki page: the package map, the startup and realtime flows, the middleware order, and the shared engines. Cite files and symbols, not line numbers, and leave out the scan ledger. Otherwise I'm fine closing this. I'll answer on #1647 too.
2 participants
Fixes #1647
What it does
Adds one file,
docs/codebase-scan-report.md: a map of the codebase onmain. Package layout, the dependency graph, how the static import and GTFS-RT paths actually flow, the route table with middleware order, where config gets read, what the tests cover. Every claim cites afile:line, so you can check it rather than trust it.No code, no behaviour change, no test changes.
Root cause
Not a bug fix. I wrote this for myself while working on the location-arrivals handlers, because I kept losing the same half hour: nearly every real-time endpoint funnels through
BuildTripStatus,arrivals_core.go,scheduled_block_helper.goandreference_utils.go, and the reasoning behind them — the Java-parity calls especially — lives only in comments beside the code. Writing it down stopped me rebuilding that model from scratch each time.Opening it in case it's useful to anyone else. #1644 fell out of it.
Testing
One Markdown file, so nothing to test. The references are checkable though, and I checked them:
mainbefore this change: n/a, docs only.I could not run the full suite — no C compiler on this machine, so CGO mode is unavailable. Section L says that outright rather than implying coverage I don't have. Nothing in the diff touches code.
Checklist
mainand passes on this branch.gtfsdb/(sqlc-generated) ortestdata/openapi.yml(synced from upstream).r.Context()to DB/service calls, notcontext.Background().testdata/openapi.yml, or an issue asking for it is open on OneBusAway/sdk-config.On the size box: 1,514 lines, and it won't split usefully because half a map is worse than none. It's additive and touches nothing else, so the review question is "do we want this, and is it accurate" rather than a diff read.
#1647 is where I've put the actual question — whether this belongs in the repo, on the wiki, or nowhere. It isn't triaged yet, so if the answer is "nowhere", say so there and I'll close this without complaint.
Notes for reviewers
Whether it belongs here at all is the real question, and I don't know the answer.
docs/README.mdsays the wiki is canonical and thatdocs/holds historical artifacts, so this is either a wiki page, a dated snapshot, or nothing. Happy to move it or close this.Two offers. Sections L and M describe my machine and what I did and didn't read, not the project — glad to cut both. And line numbers drift, so someone has to re-verify them now and then; if that's maintenance you don't want, that's a fair no.
Section K.4 is where I'd start if you're short on time: eleven doc-vs-code divergences with evidence. #1645 fixes four of them.