Count the readers who stayed, and say where they are - #249
Merged
Merged
Conversation
`readers` is a deliberately low bar -- one page outside the wall -- and a number that low moves with whatever got linked somewhere yesterday, which makes it a poor thing to compare months with. #184 wants figures that mean the same in month three as in month one. Engaged readers is the subset who went past the page they landed on: five or more page views in a day, counted OUTSIDE the wall. It is a subset rather than a new classification, so the impossible-device fingerprint and the Accept-Language test have already been applied before a visitor can reach it, and it inherits every correction those have accumulated. Three things it is careful about. Wall views do not count toward it. `views_by` includes them because `wall-once` needs them; this one must not, or a gallery visitor who scrolled five images walks back into the audience by the other door -- which is the split the wall separation exists to make. The threshold is applied per day, never pooled across a period. Over a month a crawler fetching one page a day clears five; per day it never does. It is also printed with the number, so a figure cannot outlive the definition that produced it, and ENGAGED_MIN is one place rather than scattered through the awk. Five is a judgement rather than a discovery, and the comment says so: on 2026-09-21 the wall-excluded day split into 290 addresses with one view, 145 with two to four, 57 with five to nine and 64 with ten or more, so the ambiguous band is the middle one. The comparison needs a trail, so the nightly now appends a row a day to engaged.tsv -- date, visitors, readers, engaged -- and prints the previous run's number with the change. Three columns because engaged cannot be read without the readers it is drawn from, nor those without the visitors they are filtered from. Re-running a day replaces its row instead of leaving the series with two answers for one date, and the comparison takes the row before THAT date rather than the line above, so backfilling an older day does not claim a change that never happened. One latent bug fixed on the way: `day` is 20260921 when it comes from a log's filename and 2026-09-21 when it comes from `date`. Two formats in one column would have broken the ordering the file depends on, because '-' sorts before every digit. Normalised where it is used. visitor_report now takes the counts rather than the log, because the nightly needs the same numbers twice and a second pass over a seventy-megabyte log to re-derive them is a pass nobody reading the output could account for. Tested: the subset relation at three thresholds, that wall views do not count, that the threshold is printed, the history file and its replace-on-rerun, and the comparison line. Plus `bash -n`, because most of this script is awk inside single-quoted shell strings and an apostrophe in a comment closes one -- which is how the word "today's" broke it while this was being written, and the nightly fails into a mail nobody opens. mawk and gawk agree, checked on the origin where both are installed. Refs #184.
#184 asks where the audience is, and the engaged count on its own cannot answer it. This adds the split, and compares each country with the run before in the same way the total already is. GoAccess is the only thing on the origin that can read the country database -- there is no mmdblookup and no python binding -- so the split comes out of its CSV rather than from looking addresses up directly. Three things about that format each cost a run to find, and all three are now written down beside the parse: It is CRLF, so an anchored match finds nothing at the end of a line until the carriage return is gone. Its empty columns are bare commas rather than empty quoted fields, so a split on the quote-comma does not separate them. That turns out to sort the rows for free: a continent row arrives with its first two columns welded together and a percentage in $3, while a country row splits cleanly, so testing $3 keeps the countries and drops the continents -- which would otherwise be counted a second time on top of the countries inside them, and would put "AS Asia" at the head of the list. The same welding leaves the tail of a row at ,,,"CN China", so the country is not the last field either. It is matched off the end of the line. Addresses never leave the work directory the trap removes, and only counts are printed: an address in this output would make the nightly the individual record /privacy says the site does not keep. There is a test for that, because it is the kind of thing a later change adds back by accident. By address rather than by visitor, and the comment says so -- one address carrying two engaged browsers is two engaged readers in the count above and one line here. The alternative is geolocating inside this script, which needs a database reader it does not have. A country absent from the previous day had no engaged readers that day, so absence is zero and every row carries a change. The column is dropped entirely on a first run, where each number would otherwise read as itself. `db` is now overridable through OPENIPC_GEOIP_DB, which is what makes any of this testable without the installed database, and lets a run by hand point at a copy. Checked against the real log on the origin, where it also confirms the loose thread noted on #184: Ukraine falls 63 to 4 between the 20th and the 21st, and Finland 61 to nothing, which is the /teleoperation launch spike rather than a baseline. China rises 4 to 26. mawk and gawk agree on the split, checked on the origin against that same log where both are installed. Refs #184.
PR Summary by QodoTrack engaged readers and daily country trends
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Qodo's first finding on #249, and it was right. The country split took the ADDRESSES of engaged readers and fed every log record sharing one to GoAccess, whose visitor count is per address and User-Agent. An engaged reader is an address AND a User-Agent, so the two populations were not the same: a non-engaged browser at an engaged reader's address was counted, and the fixture has exactly that shape -- one address running an engaged Firefox and a one-page iPhone. Nothing is filtered by address any more. visitors() keeps one line per visitor, that visitor's own, and hands over the lines of the engaged ones; GoAccess sees those and nothing else. The totals now reconcile with the engaged count by construction rather than by argument, and on the origin against the real log they do: 130 engaged, 130 across countries. The test asserts what GoAccess is GIVEN rather than what it returns. A stub cannot geolocate, so a stubbed total would only prove the stub was believed; the input is the thing the fix changes. Two cases: the shared address contributes the engaged browser and not the other, and when both browsers at one address qualify, both lines go over. Qodo's second finding, also fair: the country figures did not say what produced them, and #184 asks that every number name its source. The heading now carries the beacon and the database, and both series files open with a header naming their columns and method -- they outlive the report they were printed beside. That header is written rather than sorted into place. A comment character sorts ahead of a digit by byte, but the default locale collates punctuation as though it were absent and had put the line in the middle of the series. LC_ALL=C on the sort for the same reason: the order of these files is the comparison. Its third finding, that two heredoc lines exceed Layout/LineLength, is wrong. Rubocop allows long lines inside heredocs by default, master already carries 382- and 370-character log fixtures in this file, and rubocop reports no offences on either revision. Refs #184.
Five more findings from Qodo on #249, all of them real. ZERO DAYS. engaged_countries returned before touching its history when nobody was engaged, so a re-run left the previous countries standing and a genuine zero day recorded nothing at all -- after which the next run looked past it to an older, larger day and printed the difference as movement. A zero day now rewrites its rows empty, and a day whose lookup FAILED is kept distinct: it says so and leaves the rows alone, because overwriting them with nothing would record a fact nobody established. The deeper cause was that each history file decided separately what the previous run was. There is now one previous_run(), reading engaged.tsv, which has a row for every run including zero days; the country file is no longer asked a question it cannot answer. POOLED DAYS. Depth was counted per visitor across the whole log, while the threshold is documented per day. A log spanning two days turned two shallow days into one engaged one. Depth is now keyed by date and visitor and a visitor is judged on their deepest single day, which is identical for a one-day log and correct for any other. A log that spans more than one day now says so in the output. THRESHOLD CHANGES. ENGAGED_MIN is overridable and nothing recorded which value produced a number, so changing it made the next delta a comparison between two different definitions. The threshold is now stored with each row, and a comparison across a change is refused and explained rather than printed. SILENT FAILURES. `|| true` masked every goaccess failure and an empty parse looked the same as a day with no countries. Exit status is checked, stderr is surfaced, and a parse that yields nothing from a non-empty input warns that the output format may have moved. CONCURRENCY. Both histories were unlocked read-modify-write against files a hand run or a backfill can touch while cron is running. Both updates, and the read of the previous run they compare against, now happen under one flock. Verified on the origin against the real log: 133 engaged, 133 across countries, and mawk and gawk agree on the whole output. Refs #184.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #184. Two commits: the metric, then the country split.
Why
readersis a deliberately low bar — one page outside the wall — and a number that low moves with whatever got linked somewhere yesterday. #184 wants figures that mean the same thing in month three as in month one, and that one does not.Engaged readers is the subset who went past the page they landed on: five or more page views in a day, counted outside the wall.
It is a subset of the existing
readersrather than a new classification, which matters more than it sounds: the impossible-device fingerprint and the Accept-Language test have already run before a visitor can reach it, so it inherits every correction those have accumulated rather than repeating their mistakes.That output is from a real run on the origin against the actual log, not from the fixture.
What it is careful about
Wall views do not count toward it.
views_byincludes them becausewall-onceneeds them; this one must not, or a gallery visitor who scrolled five images walks back into the audience by the other door — which is the split the wall separation exists to make.The threshold is per day, never pooled. Over a month a crawler fetching one page a day clears five; per day it never does. It is printed alongside the number, so a figure cannot outlive the definition that produced it, and
ENGAGED_MINis one place rather than scattered through the awk.Five is a judgement, not a discovery, and the comment says so: on 2026-09-21 the wall-excluded day split into 290 addresses with one view, 145 with two to four, 57 with five to nine and 64 with ten or more, so the ambiguous band is the middle one.
The comparison takes the row before that date, not the line above, so backfilling an older day does not claim a change that never happened. Re-running a day replaces its row rather than leaving the series with two answers for one date.
The country split, and three traps in goaccess's CSV
GoAccess is the only thing on the origin that can read the country database — no
mmdblookup, no python binding — so the split comes out of its CSV. Each of these cost a run to find and each is now documented beside the parse:$3, a country row splits cleanly. Testing$3keeps countries and drops continents — which would otherwise be counted on top of the countries inside them, and would put "AS Asia" at the head of the list.,,,"CN China". It is matched off the end of the line.Addresses never leave the work directory the trap removes, and only counts are printed. An address in this output would make the nightly the individual record
/privacysays the site does not keep. There is a test for that specifically, because it is the kind of thing a later change adds back by accident.By address, not by visitor: one address carrying two engaged browsers is two engaged readers in the count above and one line here. The alternative is geolocating inside this script, which needs a database reader it does not have.
Incidental fixes
dayis20260921from a log filename and2026-09-21fromdate. Two formats in one column would have broken the ordering the history depends on, because-sorts before every digit. Normalised where it is used.visitor_reportnow takes the counts rather than the log, because the nightly needs the same numbers twice and a second pass over a seventy-megabyte log is one nobody reading the output could account for.dbis overridable throughOPENIPC_GEOIP_DB, which is what makes the country path testable without the installed database, and lets a run by hand point at a copy.Tested
bash -n, because most of this script is awk inside single-quoted shell strings and an apostrophe in a comment closes one. That is how the word "today's" broke it while this was being written, and the nightly fails into a mail nobody opens.One thing the real run already showed
It confirms the loose thread noted on #184: Ukraine falls 63 → 4 between the 20th and the 21st and Finland 61 → nothing, while China rises 4 → 26. That is the
/teleoperationlaunch spike rather than a baseline — exactly the distortion that would have poisoned an early reading, and it is now visible in the report itself instead of being something someone has to remember.Two days of data is not a trend, and no country ordering here should be acted on yet; the memo can start carrying this from the first full month after 20 September.