001_schema.sql now declares only what pixa writes and reads. Removed:
output_content and request_cache: nothing writes either; request_cache still pointed at the removed source_metadata.
source_content.source_host, source_path, source_query: a row is found by source_key alone, and nothing turns a key back into a URL.
source_content.content_type: processing a cached source reads only its bytes.
source_content.fetched_at: never read.
variant_content.content_type: a cache hit reads the type from the file's first bytes since #309, so reconciliation no longer opens a variant file it adopts.
variant_content.created_at: never read.
negative_cache.status_code: never read (it held 502 or 415); extractStatusCode and the status constants that only fed it go too.
negative_cache.error_message and fetched_at: never read.
Every index stays, as each serves a query that runs. The negative_cache comment now names the failures it holds; it used to name timeouts, which are not stored.
StoreSource no longer takes the fetch result, LookupSource returns only the source key, StoreVariant no longer takes a content type, and StoreNegative no longer takes a status code or message.
Tests follow the schema and the new signatures. The server test no longer checks the variant row's content type, the backdating test no longer sets fetched_at or created_at, and the two-URLs test finds each row by key only.
Judgement call: #308 listed host, path, query and fetch time for source_content; this issue asks for only what is read, so they go.
Left alone: cache_stats, which #311 replaces, and schema_migrations.applied_at (never read), which is in 000_migration.sql, outside this issue.
Model: opus-5-5
`001_schema.sql` now declares only what pixa writes and reads. Removed:
- `output_content` and `request_cache`: nothing writes either; `request_cache` still pointed at the removed `source_metadata`.
- `source_content.source_host`, `source_path`, `source_query`: a row is found by `source_key` alone, and nothing turns a key back into a URL.
- `source_content.content_type`: processing a cached source reads only its bytes.
- `source_content.fetched_at`: never read.
- `variant_content.content_type`: a cache hit reads the type from the file's first bytes since https://git.eeqj.de/sneak/pixa/issues/309, so reconciliation no longer opens a variant file it adopts.
- `variant_content.created_at`: never read.
- `negative_cache.status_code`: never read (it held 502 or 415); `extractStatusCode` and the status constants that only fed it go too.
- `negative_cache.error_message` and `fetched_at`: never read.
Every index stays, as each serves a query that runs. The `negative_cache` comment now names the failures it holds; it used to name timeouts, which are not stored.
`StoreSource` no longer takes the fetch result, `LookupSource` returns only the source key, `StoreVariant` no longer takes a content type, and `StoreNegative` no longer takes a status code or message.
Tests follow the schema and the new signatures. The server test no longer checks the variant row's content type, the backdating test no longer sets `fetched_at` or `created_at`, and the two-URLs test finds each row by key only.
- Judgement call: https://git.eeqj.de/sneak/pixa/issues/308 listed host, path, query and fetch time for `source_content`; this issue asks for only what is read, so they go.
- Left alone: `cache_stats`, which https://git.eeqj.de/sneak/pixa/issues/311 replaces, and `schema_migrations.applied_at` (never read), which is in `000_migration.sql`, outside this issue.
Model: opus-5-5
internal/db/migrations/001_schema.sql keeps source_content.content_type, but pixa never uses it. StoreSource writes it and LookupSource selects it, but the only caller of LookupSource outside tests (processFromSourceOrFetch in internal/imgcache/service.go) throws the content type away, and processing a cached source reads only its bytes. That makes it the same case as the removed host, path, query and fetched_at, and as variant_content.content_type. So the definition of done in #310 (only what pixa writes and reads) is not met, and the clause this PR adds to README.md ("a source image's row also holds its content type") describes a column that has no use. Acceptable: drop the column from the schema. StoreSource stops writing it, and stops taking the fetch result if nothing else needs it. LookupSource returns only the key, with its doc comment to match. Remove the README.md clause and update the TODO.md entry to match. Tests that pass or check that content type change only to follow the new signature.
Model: opus-5-5
- `internal/db/migrations/001_schema.sql` keeps `source_content.content_type`, but pixa never uses it. `StoreSource` writes it and `LookupSource` selects it, but the only caller of `LookupSource` outside tests (`processFromSourceOrFetch` in `internal/imgcache/service.go`) throws the content type away, and processing a cached source reads only its bytes. That makes it the same case as the removed host, path, query and `fetched_at`, and as `variant_content.content_type`. So the definition of done in https://git.eeqj.de/sneak/pixa/issues/310 (only what pixa writes and reads) is not met, and the clause this PR adds to `README.md` ("a source image's row also holds its content type") describes a column that has no use. Acceptable: drop the column from the schema. `StoreSource` stops writing it, and stops taking the fetch result if nothing else needs it. `LookupSource` returns only the key, with its doc comment to match. Remove the `README.md` clause and update the `TODO.md` entry to match. Tests that pass or check that content type change only to follow the new signature.
Model: opus-5-5
Reworked: source_content.content_type is gone from the schema, StoreSource no longer takes the fetch result and LookupSource returns only the source key, with its doc comment to match. The README.md clause is removed, the TODO.md entry updated, and tests change only to follow the new signatures.
Model: opus-5-5
Reworked: `source_content.content_type` is gone from the schema, `StoreSource` no longer takes the fetch result and `LookupSource` returns only the source key, with its doc comment to match. The `README.md` clause is removed, the `TODO.md` entry updated, and tests change only to follow the new signatures.
Model: opus-5-5
Stopped by the top-level manager at 16:36 UTC on 21 October, when the primary reached sneak's 60% guard: the independent review of this PR was running and did not report. The next manager reruns it from current next; nothing else on the branch changed.
Model: opus-5-5
Stopped by the top-level manager at 16:36 UTC on 21 October, when the primary reached sneak's 60% guard: the independent review of this PR was running and did not report. The next manager reruns it from current `next`; nothing else on the branch changed.
Model: opus-5-5
internal/imgcache/cache.go, LookupSource, line 300: the query reads source_key into foundKey, but nothing reads foundKey after that, and the function returns key. The lookup only needs to know whether the row exists. As written, a reader has to work out that the two keys are always equal and that returning key is not a mistake. This also leaves a value read from the database and never used, in a change meant to remove exactly that. Acceptable: a query that only asks whether the row exists, scanned into a variable the function then tests. For example, scan SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?) into a bool, and return the empty key when it is false, as a missing row does today.
Disclosures:
Judgement call: cache_stats, whose upstream_fetch_count, upstream_fetch_bytes, transform_count and last_updated_at are written and never read, is left to #311, which removes the whole table. I did not count it as a finding here.
Judgement call: schema_migrations.applied_at is in 000_migration.sql, which the definition of done in #310 does not cover. I did not count it as a finding either.
Model: opus-5-5
**FAIL (needs-rework)**
- `internal/imgcache/cache.go`, `LookupSource`, line 300: the query reads `source_key` into `foundKey`, but nothing reads `foundKey` after that, and the function returns `key`. The lookup only needs to know whether the row exists. As written, a reader has to work out that the two keys are always equal and that returning `key` is not a mistake. This also leaves a value read from the database and never used, in a change meant to remove exactly that. Acceptable: a query that only asks whether the row exists, scanned into a variable the function then tests. For example, scan `SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)` into a bool, and return the empty key when it is false, as a missing row does today.
Disclosures:
- Judgement call: `cache_stats`, whose `upstream_fetch_count`, `upstream_fetch_bytes`, `transform_count` and `last_updated_at` are written and never read, is left to https://git.eeqj.de/sneak/pixa/issues/311, which removes the whole table. I did not count it as a finding here.
- Judgement call: `schema_migrations.applied_at` is in `000_migration.sql`, which the definition of done in https://git.eeqj.de/sneak/pixa/issues/310 does not cover. I did not count it as a finding either.
Model: opus-5-5
Removes output_content and request_cache, which nothing writes, and every
column that was written but never read: the host, path, query, content_type
and fetched_at of source_content, the content_type and created_at of
variant_content, and the status_code, error_message and fetched_at of
negative_cache.
StoreSource no longer takes the fetch result and LookupSource returns only
the source key. StoreVariant no longer takes a content type and
StoreNegative no longer takes a status code or message, so
extractStatusCode and the status constants, which only fed status_code,
are gone. Reconciliation adopts a variant file without opening it, as its
row no longer records a content type. Tests that set or checked a removed
column follow.
Model: opus-5-5
LookupSource now scans SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?) into a bool and returns the empty key when it is false; nothing else changed.
Model: opus-5-5
`LookupSource` now scans `SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)` into a bool and returns the empty key when it is false; nothing else changed.
Model: opus-5-5
PASS: 001_schema.sql declares only what pixa writes and reads, leaving cache_stats to #311, and the code, tests and README.md match it.
Judgement call: the id column of cache_usage, cache_stats and logout always holds 1, but it is what keeps each of those tables to one row, so I did not count it as a column that only ever receives one value.
Model: opus-5-5
PASS: `001_schema.sql` declares only what pixa writes and reads, leaving `cache_stats` to https://git.eeqj.de/sneak/pixa/issues/311, and the code, tests and `README.md` match it.
- Judgement call: the `id` column of `cache_usage`, `cache_stats` and `logout` always holds 1, but it is what keeps each of those tables to one row, so I did not count it as a column that only ever receives one value.
Model: opus-5-5
clawbot
merged commit 6b19e4c96c into next2026-10-26 09:29:27 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
001_schema.sqlnow declares only what pixa writes and reads. Removed:output_contentandrequest_cache: nothing writes either;request_cachestill pointed at the removedsource_metadata.source_content.source_host,source_path,source_query: a row is found bysource_keyalone, and nothing turns a key back into a URL.source_content.content_type: processing a cached source reads only its bytes.source_content.fetched_at: never read.variant_content.content_type: a cache hit reads the type from the file's first bytes since #309, so reconciliation no longer opens a variant file it adopts.variant_content.created_at: never read.negative_cache.status_code: never read (it held 502 or 415);extractStatusCodeand the status constants that only fed it go too.negative_cache.error_messageandfetched_at: never read.Every index stays, as each serves a query that runs. The
negative_cachecomment now names the failures it holds; it used to name timeouts, which are not stored.StoreSourceno longer takes the fetch result,LookupSourcereturns only the source key,StoreVariantno longer takes a content type, andStoreNegativeno longer takes a status code or message.Tests follow the schema and the new signatures. The server test no longer checks the variant row's content type, the backdating test no longer sets
fetched_atorcreated_at, and the two-URLs test finds each row by key only.source_content; this issue asks for only what is read, so they go.cache_stats, which #311 replaces, andschema_migrations.applied_at(never read), which is in000_migration.sql, outside this issue.Model: opus-5-5
clawbot referenced this pull request2026-10-21 18:25:17 +02:00
internal/db/migrations/001_schema.sqlkeepssource_content.content_type, but pixa never uses it.StoreSourcewrites it andLookupSourceselects it, but the only caller ofLookupSourceoutside tests (processFromSourceOrFetchininternal/imgcache/service.go) throws the content type away, and processing a cached source reads only its bytes. That makes it the same case as the removed host, path, query andfetched_at, and asvariant_content.content_type. So the definition of done in #310 (only what pixa writes and reads) is not met, and the clause this PR adds toREADME.md("a source image's row also holds its content type") describes a column that has no use. Acceptable: drop the column from the schema.StoreSourcestops writing it, and stops taking the fetch result if nothing else needs it.LookupSourcereturns only the key, with its doc comment to match. Remove theREADME.mdclause and update theTODO.mdentry to match. Tests that pass or check that content type change only to follow the new signature.Model: opus-5-5
a6eba6045eto03b11e9703Reworked:
source_content.content_typeis gone from the schema,StoreSourceno longer takes the fetch result andLookupSourcereturns only the source key, with its doc comment to match. TheREADME.mdclause is removed, theTODO.mdentry updated, and tests change only to follow the new signatures.Model: opus-5-5
Stopped by the top-level manager at 16:36 UTC on 21 October, when the primary reached sneak's 60% guard: the independent review of this PR was running and did not report. The next manager reruns it from current
next; nothing else on the branch changed.Model: opus-5-5
FAIL (needs-rework)
internal/imgcache/cache.go,LookupSource, line 300: the query readssource_keyintofoundKey, but nothing readsfoundKeyafter that, and the function returnskey. The lookup only needs to know whether the row exists. As written, a reader has to work out that the two keys are always equal and that returningkeyis not a mistake. This also leaves a value read from the database and never used, in a change meant to remove exactly that. Acceptable: a query that only asks whether the row exists, scanned into a variable the function then tests. For example, scanSELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)into a bool, and return the empty key when it is false, as a missing row does today.Disclosures:
cache_stats, whoseupstream_fetch_count,upstream_fetch_bytes,transform_countandlast_updated_atare written and never read, is left to #311, which removes the whole table. I did not count it as a finding here.schema_migrations.applied_atis in000_migration.sql, which the definition of done in #310 does not cover. I did not count it as a finding either.Model: opus-5-5
03b11e9703to0742446199LookupSourcenow scansSELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)into a bool and returns the empty key when it is false; nothing else changed.Model: opus-5-5
PASS:
001_schema.sqldeclares only what pixa writes and reads, leavingcache_statsto #311, and the code, tests andREADME.mdmatch it.idcolumn ofcache_usage,cache_statsandlogoutalways holds 1, but it is what keeps each of those tables to one row, so I did not count it as a column that only ever receives one value.Model: opus-5-5