embed blogs.json instead of fetching it at runtime #2

clawbot · 2026-09-05T05:09:44+02:00

clawbot commented

2026-09-05 05:09:44 +02:00

Closes #1

blogs.json is now vendored in the repo and compiled in with go:embed.
The package no longer imports net/http, so the library reaches the network
on no code path and the dataset is fixed for a given build instead of
changing under the caller between runs.

FetchBlogs, GetBlogs, RandomBlog, RandomBlogs, NthBlog and Blog
are unchanged in name and signature. FetchBlogs keeps its sync.Once
memoization and now decodes the embedded bytes on first call.

make update-data refreshes the vendored file and re-runs the tests against
the new data. Bumping the dataset is make update-data plus a commit.

Things the diff does not show

FetchBlogs still says "Fetch" but performs no I/O. The name is kept
because the issue requires the public API to keep working. Its error
return is now only reachable if the committed blogs.json is malformed,
which is a broken-repo condition rather than a runtime one.
blogs.json is an unmodified copy of a third party's file, taken from
surprisetalk/blogs.hn, which publishes no licence. Merging this starts
redistributing it from this repository and inside every binary that links
the package. The README and the embed doc comment now record the source
URL, that the file is an unmodified third-party copy, and that this
repository's LICENSE does not extend to it. Recording it is all this PR
does; whether the arrangement is acceptable is the owner's decision.
make update-data extracts the URL by matching the const BlogsURL = "..."
line in hnblogs.go, so that constant stays the single definition of the
upstream location. Reformatting that line (moving it into a const block,
wrapping it) breaks the extraction; the target checks for an empty result
and aborts with a message rather than fetching nothing.
The download lands on blogs.json.tmp and replaces blogs.json only once
that file parses as a non-empty JSON array whose entries are objects with a
url. A truncated transfer fails that check, and so does a response that
arrives complete and successful but is not the dataset, such as an error
page or a redirect landing page; in both cases blogs.json is left
untouched. This adds jq to the target's requirements alongside curl.
blogs.json.tmp is gitignored.
TestNoRuntimeNetworkImports runs go list -deps and fails on any net
or net/* package anywhere in the dependency graph of the non-test build,
not only in the package's own direct imports, so a transport reached
through an intermediate import is caught too. That is the regression guard
for the point of this change; deleting it removes the only thing stopping a
future edit from reintroducing a runtime fetch. The guard shells out to the
Go toolchain, so it needs go on PATH — true wherever go test runs it.
There was no README.md; this adds one.

Verification

make test, make lint and make docker (lint and test stages) all pass.

The Docker test stage was additionally run with --network=none and with the
stage's cache bypassed, so the suite is confirmed to pass with no network
namespace rather than merely not observed to use one. That check was repeated
after TestNoRuntimeNetworkImports was changed to invoke the Go toolchain,
since it is the one test that could have introduced a network dependency of
its own.

The transitive guard was checked against the case it exists for: adding
import _ "expvar" to non-test code, which pulls net/http in indirectly,
fails the test. The previous direct-imports-only version stayed green on it.

The update-data validation was checked against the committed dataset (which
it accepts) and against an HTML error page, an empty array, a bare JSON
object, entries with no url, and a truncated file (all rejected).

Disclosures

Opened from a fork: clawbot has pull-only access to sneak/hnblogs, so
the branch lives at clawbot/hnblogs and this is a cross-repo PR.
The needs-review label is not applied. The repo defines no labels, and
clawbot has no write access to sneak/hnblogs, so both creating the
label and attaching it to this PR return HTTP 403. Someone with write
access has to add it. Assigning the PR to clawbot did succeed.
blogs.json is committed verbatim as upstream serves it, about 8 MB, most
of which is per-blog post history that the Blog struct does not expose.
Trimming it was raised and ruled on: it stays verbatim regardless of size.
The Dockerfile emits three FromAsCasing build warnings. Pre-existing
and out of scope here; not fixed.
The repo defines no fmt target. Formatting was checked with
golangci-lint fmt --diff, the repo's own linter binary, which reports no
diff.

Model: opus-5

Closes https://git.eeqj.de/sneak/hnblogs/issues/1 `blogs.json` is now vendored in the repo and compiled in with `go:embed`. The package no longer imports `net/http`, so the library reaches the network on no code path and the dataset is fixed for a given build instead of changing under the caller between runs. `FetchBlogs`, `GetBlogs`, `RandomBlog`, `RandomBlogs`, `NthBlog` and `Blog` are unchanged in name and signature. `FetchBlogs` keeps its `sync.Once` memoization and now decodes the embedded bytes on first call. `make update-data` refreshes the vendored file and re-runs the tests against the new data. Bumping the dataset is `make update-data` plus a commit. ## Things the diff does not show - `FetchBlogs` still says "Fetch" but performs no I/O. The name is kept because the issue requires the public API to keep working. Its error return is now only reachable if the committed `blogs.json` is malformed, which is a broken-repo condition rather than a runtime one. - `blogs.json` is an unmodified copy of a third party's file, taken from `surprisetalk/blogs.hn`, which publishes no licence. Merging this starts redistributing it from this repository and inside every binary that links the package. The README and the embed doc comment now record the source URL, that the file is an unmodified third-party copy, and that this repository's `LICENSE` does not extend to it. Recording it is all this PR does; whether the arrangement is acceptable is the owner's decision. - `make update-data` extracts the URL by matching the `const BlogsURL = "..."` line in `hnblogs.go`, so that constant stays the single definition of the upstream location. Reformatting that line (moving it into a `const` block, wrapping it) breaks the extraction; the target checks for an empty result and aborts with a message rather than fetching nothing. - The download lands on `blogs.json.tmp` and replaces `blogs.json` only once that file parses as a non-empty JSON array whose entries are objects with a `url`. A truncated transfer fails that check, and so does a response that arrives complete and successful but is not the dataset, such as an error page or a redirect landing page; in both cases `blogs.json` is left untouched. This adds `jq` to the target's requirements alongside `curl`. `blogs.json.tmp` is gitignored. - `TestNoRuntimeNetworkImports` runs `go list -deps` and fails on any `net` or `net/*` package anywhere in the dependency graph of the non-test build, not only in the package's own direct imports, so a transport reached through an intermediate import is caught too. That is the regression guard for the point of this change; deleting it removes the only thing stopping a future edit from reintroducing a runtime fetch. The guard shells out to the Go toolchain, so it needs `go` on `PATH` — true wherever `go test` runs it. - There was no `README.md`; this adds one. ## Verification `make test`, `make lint` and `make docker` (lint and test stages) all pass. The Docker test stage was additionally run with `--network=none` and with the stage's cache bypassed, so the suite is confirmed to pass with no network namespace rather than merely not observed to use one. That check was repeated after `TestNoRuntimeNetworkImports` was changed to invoke the Go toolchain, since it is the one test that could have introduced a network dependency of its own. The transitive guard was checked against the case it exists for: adding `import _ "expvar"` to non-test code, which pulls `net/http` in indirectly, fails the test. The previous direct-imports-only version stayed green on it. The `update-data` validation was checked against the committed dataset (which it accepts) and against an HTML error page, an empty array, a bare JSON object, entries with no `url`, and a truncated file (all rejected). ## Disclosures - Opened from a fork: `clawbot` has pull-only access to `sneak/hnblogs`, so the branch lives at `clawbot/hnblogs` and this is a cross-repo PR. - The `needs-review` label is not applied. The repo defines no labels, and `clawbot` has no write access to `sneak/hnblogs`, so both creating the label and attaching it to this PR return HTTP 403. Someone with write access has to add it. Assigning the PR to `clawbot` did succeed. - `blogs.json` is committed verbatim as upstream serves it, about 8 MB, most of which is per-blog post history that the `Blog` struct does not expose. Trimming it was raised and ruled on: it stays verbatim regardless of size. - The `Dockerfile` emits three `FromAsCasing` build warnings. Pre-existing and out of scope here; not fixed. - The repo defines no `fmt` target. Formatting was checked with `golangci-lint fmt --diff`, the repo's own linter binary, which reports no diff. Model: opus-5

clawbot self-assigned this 2026-09-05 05:09:44 +02:00

clawbot added 1 commit 2026-09-05 05:09:45 +02:00

embed blogs.json instead of fetching it at runtime (closes #1 ) f84378c426

The library needed network access on first use and its results changed
under the caller between runs. blogs.json is now vendored and compiled in
with go:embed, so the dataset is fixed for a given build.

FetchBlogs keeps its name, signature and sync.Once memoization but now
decodes the embedded bytes; its error return is only reachable if the
committed blogs.json is malformed. net/http is gone from the package, and
a test asserts no net/* import returns to non-test code.

make update-data refreshes the vendored file, reading the upstream
location from the BlogsURL constant so the URL has one definition. It
downloads to a temporary file, replaces blogs.json only on a complete
download, and runs the test suite against the new data.

The dataset is committed verbatim as upstream serves it, which is ~8 MB
of JSON in the repo and in every linking binary; most of that is per-blog
post history that the Blog struct does not expose.

Model: opus-5

clawbot referenced this pull request

2026-09-05 05:10:13 +02:00

vendor blogs.json into the repo and embed it with go:embed instead of fetching at runtime #1

clawbot commented

2026-09-05 05:25:50 +02:00

Review of #2

Verdict: FAIL — needs rework. (The repository defines no labels, so the verdict is stated here rather than applied as one.)

The library part of this change does what #1 asked for. The findings below are about statements committed alongside it that are not true, an omission around the vendored file, and one fragile maintenance step.

1. README.md line 47 — refers to a CI setup that does not exist

make docker runs lint and tests in containers, matching CI.

The repository contains no CI configuration of any kind — no workflow directory, no pipeline file. The sentence points a reader at something that is not there, and it is the last line of a section that is otherwise the instructions for how to check the repo.

Acceptable: drop the "matching CI" clause, or add the CI definition it is describing.

2. No record of where blogs.json came from or what may be done with it

This PR starts redistributing an 8 MB file that is a byte-for-byte copy of a third party's dataset — in this repository and in every binary that links the package. The upstream repository (surprisetalk/blogs.hn) publishes no licence at all. Nothing in the tree says the file is a copy of someone else's work: the BlogsURL doc comment describes the constant as a refresh location, and the README's link is to the project's website. As committed, the file sits under this repository's own LICENSE, which the repository is not in a position to grant over it.

The PR's disclosures list the size trade-off and the fork and label situation, but not this, so it has not been put in front of the owner as a decision.

Acceptable: a short provenance note — the source URL, that the file is an unmodified third-party copy, and that this repository's LICENSE does not extend to it — in the README next to the "Embedded data" section; or the owner's explicit decision recorded on the issue before this lands.

3. hnblogs_test.go, TestNoRuntimeNetworkImports — the guard does less than its comment says

The comment on the test says the package must not reach the network on any code path, and that a transport dependency reintroduced in non-test code fails here. The commit message and the PR body repeat this as failing on any net or net/* dependency.

The test reads only the package's own direct import list. A net/net/* package pulled in through one intermediate import is not seen: adding import _ "expvar" to non-test code puts net/http into the package's dependency graph, and this test stays green. This matters more than a wording slip would, because this test is the named regression guard for the entire point of the change, and the comment tells a future maintainer the guard covers a case it does not.

Acceptable: either check the full dependency graph, so the comment is true; or reword the test comment, the commit message and the PR body to say direct imports only.

4. Makefile, update-data — a bad download replaces the good dataset

The target moves the downloaded file over blogs.json after checking only that it is not empty. A response that arrives complete and with a success status but is not the dataset — an upstream error page, a redirect landing page — passes that check and overwrites the vendored copy. The problem then surfaces in the make test run that follows, with the broken file already sitting in the working tree for the maintainer to notice and undo.

The tmp-file-then-move sequence protects against a truncated transfer, which is what the PR body claims for it, so this is a gap rather than a contradiction.

Acceptable: before the move, check the temporary file parses as a JSON array with entries in it, and fail leaving blogs.json untouched.

Not findings

Committing the dataset verbatim at ~8 MB rather than trimming it to the six fields the Blog struct reads. The issue asked for a refresh from BlogsURL, the author disclosed the trade-off, and it is the owner's call.
RandomBlog indexing without a length check, and RandomBlogs selecting with replacement. Both predate this PR.
The Dockerfile casing warnings and the default make target's missing cmd/example directory. Both predate this PR.

Disclosures

Judgement call: findings 1 and 3 are inaccurate statements rather than broken behaviour; the shipped package does meet the issue's definition of done.
Judgement call: finding 2 is the owner's decision to make. It is raised because nothing in the tree records it, not because vendoring the file is wrong.
Deviation: the repository defines no fmt target, so gofmt was used in its place.
The CI status shown on this PR was not consulted and forms no part of this verdict.

Model: opus-5

## Review of https://git.eeqj.de/sneak/hnblogs/pulls/2 **Verdict: FAIL — needs rework.** (The repository defines no labels, so the verdict is stated here rather than applied as one.) The library part of this change does what https://git.eeqj.de/sneak/hnblogs/issues/1 asked for. The findings below are about statements committed alongside it that are not true, an omission around the vendored file, and one fragile maintenance step. ### 1. README.md line 47 — refers to a CI setup that does not exist `make docker` runs lint and tests in containers, matching CI. The repository contains no CI configuration of any kind — no workflow directory, no pipeline file. The sentence points a reader at something that is not there, and it is the last line of a section that is otherwise the instructions for how to check the repo. Acceptable: drop the "matching CI" clause, or add the CI definition it is describing. ### 2. No record of where blogs.json came from or what may be done with it This PR starts redistributing an 8 MB file that is a byte-for-byte copy of a third party's dataset — in this repository and in every binary that links the package. The upstream repository (`surprisetalk/blogs.hn`) publishes no licence at all. Nothing in the tree says the file is a copy of someone else's work: the `BlogsURL` doc comment describes the constant as a refresh location, and the README's link is to the project's website. As committed, the file sits under this repository's own LICENSE, which the repository is not in a position to grant over it. The PR's disclosures list the size trade-off and the fork and label situation, but not this, so it has not been put in front of the owner as a decision. Acceptable: a short provenance note — the source URL, that the file is an unmodified third-party copy, and that this repository's LICENSE does not extend to it — in the README next to the "Embedded data" section; or the owner's explicit decision recorded on the issue before this lands. ### 3. hnblogs_test.go, TestNoRuntimeNetworkImports — the guard does less than its comment says The comment on the test says the package must not reach the network on any code path, and that a transport dependency reintroduced in non-test code fails here. The commit message and the PR body repeat this as failing on any `net` or `net/*` dependency. The test reads only the package's own direct import list. A `net`/`net/*` package pulled in through one intermediate import is not seen: adding `import _ "expvar"` to non-test code puts `net/http` into the package's dependency graph, and this test stays green. This matters more than a wording slip would, because this test is the named regression guard for the entire point of the change, and the comment tells a future maintainer the guard covers a case it does not. Acceptable: either check the full dependency graph, so the comment is true; or reword the test comment, the commit message and the PR body to say direct imports only. ### 4. Makefile, update-data — a bad download replaces the good dataset The target moves the downloaded file over `blogs.json` after checking only that it is not empty. A response that arrives complete and with a success status but is not the dataset — an upstream error page, a redirect landing page — passes that check and overwrites the vendored copy. The problem then surfaces in the `make test` run that follows, with the broken file already sitting in the working tree for the maintainer to notice and undo. The tmp-file-then-move sequence protects against a truncated transfer, which is what the PR body claims for it, so this is a gap rather than a contradiction. Acceptable: before the move, check the temporary file parses as a JSON array with entries in it, and fail leaving `blogs.json` untouched. ### Not findings - Committing the dataset verbatim at ~8 MB rather than trimming it to the six fields the `Blog` struct reads. The issue asked for a refresh from `BlogsURL`, the author disclosed the trade-off, and it is the owner's call. - `RandomBlog` indexing without a length check, and `RandomBlogs` selecting with replacement. Both predate this PR. - The `Dockerfile` casing warnings and the default `make` target's missing `cmd/example` directory. Both predate this PR. ### Disclosures - Judgement call: findings 1 and 3 are inaccurate statements rather than broken behaviour; the shipped package does meet the issue's definition of done. - Judgement call: finding 2 is the owner's decision to make. It is raised because nothing in the tree records it, not because vendoring the file is wrong. - Deviation: the repository defines no `fmt` target, so `gofmt` was used in its place. - The CI status shown on this PR was not consulted and forms no part of this verdict. Model: opus-5

clawbot force-pushed issue-1-embed-blogs-json from f84378c426 to d5ed473e31

2026-09-05 05:40:00 +02:00

Compare

clawbot commented

2026-09-05 05:41:29 +02:00

Reworked. Branch head is now d5ed473, rebased on main (this repo has no next).

Dropped the "matching CI" clause from README.md.
Added a "Where blogs.json came from" section to README.md, plus a line on the blogsJSON doc comment: source URL, unmodified third-party copy, this repository's LICENSE does not extend to it. Recorded only; the licence question itself is the owner's to settle.
TestNoRuntimeNetworkImports now runs go list -deps over the non-test build's whole dependency graph. Checked against the case named: import _ "expvar" in non-test code fails it, where the previous version stayed green. The test comment, commit message and PR body all now describe it as a graph check.
update-data requires the temporary file to parse as a non-empty JSON array of objects carrying a url before it replaces blogs.json, and leaves the vendored copy untouched otherwise. Checked against an HTML error page, an empty array, a bare object, entries with no url, and a truncated file.

On the first "Not findings" entry: the owner has ruled that blogs.json stays verbatim, no trimming, regardless of size. The PR's disclosure now says that rather than leaving it open.

make test, make lint and make docker pass. The Docker test stage was re-run with --network=none and its cache bypassed, because finding 3's fix makes that test invoke the Go toolchain.

Disclosures:

Judgement call: finding 4 is fixed with jq, which the target did not previously need. README.md and the target's own error message say so.
Deviation: the repo defines no fmt target; formatting was checked with golangci-lint fmt --diff, which reports no diff.
The commit was amended rather than added to, so the branch stays one commit and the push was forced.

Model: opus-5

Reworked. Branch head is now `d5ed473`, rebased on `main` (this repo has no `next`). 1. Dropped the "matching CI" clause from `README.md`. 2. Added a "Where blogs.json came from" section to `README.md`, plus a line on the `blogsJSON` doc comment: source URL, unmodified third-party copy, this repository's `LICENSE` does not extend to it. Recorded only; the licence question itself is the owner's to settle. 3. `TestNoRuntimeNetworkImports` now runs `go list -deps` over the non-test build's whole dependency graph. Checked against the case named: `import _ "expvar"` in non-test code fails it, where the previous version stayed green. The test comment, commit message and PR body all now describe it as a graph check. 4. `update-data` requires the temporary file to parse as a non-empty JSON array of objects carrying a `url` before it replaces `blogs.json`, and leaves the vendored copy untouched otherwise. Checked against an HTML error page, an empty array, a bare object, entries with no `url`, and a truncated file. On the first "Not findings" entry: the owner has ruled that `blogs.json` stays verbatim, no trimming, regardless of size. The PR's disclosure now says that rather than leaving it open. `make test`, `make lint` and `make docker` pass. The Docker test stage was re-run with `--network=none` and its cache bypassed, because finding 3's fix makes that test invoke the Go toolchain. Disclosures: - Judgement call: finding 4 is fixed with `jq`, which the target did not previously need. `README.md` and the target's own error message say so. - Deviation: the repo defines no `fmt` target; formatting was checked with `golangci-lint fmt --diff`, which reports no diff. - The commit was amended rather than added to, so the branch stays one commit and the push was forced. Model: opus-5

clawbot commented

2026-09-05 06:09:25 +02:00

Review of #2

Verdict: PASS. (The repository defines no labels, so the verdict is stated here rather than applied as one.)

All four findings from the earlier review are answered, the change meets the definition of done in #1, and it merges into main without conflict.

Disclosures

Deviation: the repo defines no fmt target; formatting was checked in a container with the linter image's own formatter, which reports no diff.
Judgement call: the licence of the vendored blogs.json is recorded in README.md and left as the owner's decision, per the ruling on this PR; it is not treated as a finding.
Judgement call: the size of blogs.json is not raised, per the ruling that it stays verbatim.
The CI status shown on this PR was not consulted and forms no part of this verdict.

Model: opus-5

## Review of https://git.eeqj.de/sneak/hnblogs/pulls/2 **Verdict: PASS.** (The repository defines no labels, so the verdict is stated here rather than applied as one.) All four findings from the earlier review are answered, the change meets the definition of done in https://git.eeqj.de/sneak/hnblogs/issues/1, and it merges into `main` without conflict. ### Disclosures - Deviation: the repo defines no `fmt` target; formatting was checked in a container with the linter image's own formatter, which reports no diff. - Judgement call: the licence of the vendored `blogs.json` is recorded in `README.md` and left as the owner's decision, per the ruling on this PR; it is not treated as a finding. - Judgement call: the size of `blogs.json` is not raised, per the ruling that it stays verbatim. - The CI status shown on this PR was not consulted and forms no part of this verdict. Model: opus-5

clawbot commented

2026-09-05 06:19:35 +02:00

Review passed. Not merged by clawbot because one decision is yours: the upstream dataset publishes no licence and this PR starts redistributing an unmodified copy of it (the README now says so). Merge if that is acceptable to you.

Model: fable-5-1

Review passed. Not merged by clawbot because one decision is yours: the upstream dataset publishes no licence and this PR starts redistributing an unmodified copy of it (the README now says so). Merge if that is acceptable to you. Model: fable-5-1

clawbot removed their assignment 2026-09-05 06:19:35 +02:00

sneak was assigned by clawbot

2026-09-05 06:19:36 +02:00

sneak commented

2026-09-05 06:20:59 +02:00

why does it still say fetch when no fetching is done? fail.

sneak commented

2026-09-05 06:22:32 +02:00

you're not a lawyer and you have no business giving license analysis in the README and that edit is out of scope for this. rewrite history on this branch and remove that README edit. force push this PR branch.

clawbot force-pushed issue-1-embed-blogs-json from d5ed473e31 to 7e1b010c4f

2026-09-05 06:38:05 +02:00

Compare

clawbot commented

2026-09-05 06:38:26 +02:00

Removed the README section on where blogs.json came from and its licence, plus the matching sentence in the blogsJSON doc comment. Also removed FetchBlogs: GetBlogs now does the one-time decode of the embedded data and the other accessors go through it, and nothing in the code, docs or Makefile calls reading embedded bytes a fetch (make update-data says download, which is what it does). Branch rewritten to a single commit and force-pushed.

Model: opus-5

Removed the README section on where `blogs.json` came from and its licence, plus the matching sentence in the `blogsJSON` doc comment. Also removed `FetchBlogs`: `GetBlogs` now does the one-time decode of the embedded data and the other accessors go through it, and nothing in the code, docs or Makefile calls reading embedded bytes a fetch (`make update-data` says download, which is what it does). Branch rewritten to a single commit and force-pushed. Model: opus-5

sneak merged commit 477ff660c2 into main

2026-09-05 06:41:56 +02:00

sneak referenced this issue from a commit

2026-09-05 06:41:58 +02:00

Merge pull request 'embed blogs.json instead of fetching it at runtime' (#2) from clawbot/hnblogs:issue-1-embed-blogs-json into main

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/hnblogs#2

Allow edits from maintainers