AG-58737 Fix incorrect "Closed as stale" count in stats report by slvvko · Pull Request #32 · AdguardTeam/AdGuardFiltersStats

slvvko · 2026-09-08T18:25:17Z

AG-58737

Fixes the incorrect Closed as stale count in the Filters stats collector report.

Problem

The Closed as stale metric counted every issue closed while carrying the Stale label, regardless of who closed it. The stale bot only adds the Stale label; maintainers routinely close those labelled issues manually during triage, so the metric systematically over-reported bot closures (e.g. 18 on 07.09.2026 when the bot closed nothing, and 118 of 119 stale-labelled closes over the last 30 days were manual).

Fix

src/tools/events-utils.js: new isStaleBotActor predicate — an issue close counts as "closed as stale" only when a stale bot account (adguard-bot, github-actions[bot], i.e. EXCLUDED_USERNAMES) closed a Stale-labelled issue. Contributor-level Resolved issues credits human closes of stale-labelled issues consistently with the repo-level metric (bot closes are excluded via EXCLUDED_USERNAMES).
src/prepare-stats/prepare-repo-stat.js: Closed as stale and Resolved issues are computed from the partition above — every close lands in exactly one counter: a maintainer closing a stale-labelled issue during triage counts as resolved, and so does a bot close of a non-stale-labelled issue.
Tests updated and extended: human close of a stale issue → resolved; stale-bot closes (both bot accounts) → closed as stale and never credited to a contributor; stale-bot close of a non-stale issue → resolved.
The Slack "what is it?" legend link now points to the README "Activity points" section (the old anchor did not exist).
CHANGELOG.md updated (v1.1.1), package.json bumped to 1.1.1.

Merge conflict resolution

Merged master and resolved the conflicts. All conflicts were in the generated Rollup bundles under bin/, which were rebuilt from src/ by the poll-events bot and dragged into every "Events collection update" commit. Since every CI workflow runs yarn build before invoking the CLI, bin/ is now fully gitignored and untracked (previously only the hashed chunks were ignored) — this removes the recurring conflict source for open PRs. AGENTS.md and DEVELOPMENT.md updated to reflect the new policy.

Dropped npm `bin` field, renamed build dir `bin/` → `dist/`

The package was published to npm only once (1.0.0) and is not published anymore, so the bin field (which only creates npm install symlinks) is vestigial. Removed it from package.json and renamed the Rollup output directory from bin/ to dist/ to follow the standard convention for generated build output. package.json scripts, .gitignore, .eslintignore, AGENTS.md, and DEVELOPMENT.md updated accordingly.

⚠️ let's remove the package from npm https://www.npmjs.com/package/@adguard/github-stats, shall we? (will be done separately)

Verification

yarn lint passes, yarn test 38/38 pass, yarn build succeeds (outputs to dist/).
Re-running the new logic against the 07.09.2026 collection: Closed as stale: 0 (was 18), Resolved: 180.

Review fixes (2026-09-10)

6b0b8011a — addresses the [P2] review comment: yarn run publish everywhere the Slack publish script is invoked from docs and examples (previously yarn publish fell back to Yarn Classic's built-in npm publish command). Scoped lines + identical occurrences in AGENTS.md / DEVELOPMENT.md.
94e641dad — addresses the [P3] review comment: AGENTS.md no longer links to the gitignored dist/ directory; it is rendered as inline code.
30efa131b — fixes a pre-existing markdownlint (MD060) table misalignment in AGENTS.md found while addressing the review.

Resolve bin/ conflicts by untracking the generated Rollup bundles: they are rebuilt by 'yarn build' in every CI workflow, so committed copies only go stale and cause recurring merge conflicts.

The package is not published to npm anymore, so the 'bin' field is vestigial (it only creates npm install symlinks). Remove it and rename the Rollup output directory to dist/ to match the standard convention for generated build output. Update package.json scripts, .gitignore, .eslintignore, AGENTS.md, and DEVELOPMENT.md accordingly.

slvvko

Fresh-eyes pass over the changes — mostly small things, the ones that matter are the actor guard, the close-partition gap, and the npm/dist consequences. Will address them in a follow-up commit.

slvvko · 2026-09-09T20:44:48Z

README.md:69-79 and examples/*.yaml still document npm i -g @adguard/github-stats / npx @adguard/github-stats as the install path, but with the bin field dropped those commands won't exist for any future release — the docs would silently break for downstream users. Switch them to the build-from-source flow (git clone + yarn install + yarn build + yarn poll|stats|publish), or keep the bin field until the package is actually removed from npm.

slvvko · 2026-09-09T20:44:48Z

README.md:35 still says activity points include "Issue closed (unless marked as Stale)" — that's the old rule. getActivityAuthor now credits any close of a stale-labelled issue; bot closes are filtered by username, not by the label. Worth rewording so the docs match the fix.

slvvko · 2026-09-09T20:44:49Z

Quick check on the PR description: "134 of 135 stale-labelled closes over the last 30 days were manual" — the local collection (30 daily files, 2026-08-11..2026-09-09) shows 119 stale-labelled closes, 118 manual + 1 bot. The other figures (18 → 0, Resolved 180) reproduce exactly. Probably a different window or REST-reconcile data; worth correcting the number in the description.

slvvko · 2026-09-09T20:44:49Z

nit: the getActivityAuthor close semantics changed in this PR (any CLOSED now credits the actor), but the unit test only covers the bot case — a human close of a stale-labelled issue returning the username is only exercised via the integration test.

- Use EXCLUDED_USERNAMES for the stale-bot actor check instead of a duplicated STALE_BOT_USERNAMES list; move the predicate to events-utils next to the other event predicates and guard against a null actor. - Make 'resolved' and 'closed as stale' exact complements so bot closes of non-stale-labelled issues are counted as resolutions instead of being dropped; cover bot variants and contributor-level exclusion in tests. - Add prepoll/prestats/prepublish build hooks; update README and examples to the build-from-source flow now that the npm bin field is gone.

slvvko · 2026-09-09T20:48:00Z

All four points addressed in d32c3a8:

README and examples/*.yaml now use the build-from-source flow (git clone + yarn install + yarn build + yarn poll|stats|publish) instead of npm i -g.
The activity-points rule is reworded — bot closes, not the Stale label, are what's excluded.
The description now says "118 of 119 stale-labelled closes" (verified against the local collection).
A unit test for a human close of a stale-labelled issue was added to events-utils.test.js.

The link pointed to a nonexistent '#github-stats-cli-app' anchor; the 'Activity points' section carries the 'activity_count' anchor.

slvvko · 2026-09-09T22:06:18Z

@maximtop let's remove the package from npm https://www.npmjs.com/package/@adguard/github-stats, shall we? it was deployed only once and never since that + works fine as is (within the repo)

Every CI workflow runs 'yarn build' explicitly before invoking the CLIs, and the local flow is documented in DEVELOPMENT.md, so the pre-hooks add nothing. 'prepublish' additionally fires on every 'yarn install' with Yarn Classic, needlessly rebuilding during installs.

maximtop

Please correct the Slack publish command in the example and README, and replace the broken link to the generated dist directory.

maximtop

lgtm — one small suggestion inline.

Extract the duplicated stale-close check into a single isClosedAsStale predicate in events-utils.js and derive both repo-level counters from it and its negation, so the partition cannot drift out of sync. Add unit tests for the predicate covering bot/human x stale/non-stale closes.

AG-58737 count only stale bot closes as closed as stale

14aa1dc

slvvko mentioned this pull request Sep 8, 2026

AG-58751 Add CI checks for pull requests #33

Merged

slvvko added 4 commits September 8, 2026 18:22

Merge branch 'master' into fix/AG-58737

d47d1ed

Merge branch 'master' into fix/AG-58737

4885f12

Resolve bin/ conflicts by untracking the generated Rollup bundles: they are rebuilt by 'yarn build' in every CI workflow, so committed copies only go stale and cause recurring merge conflicts.

update gitignore

6d5ff0a

slvvko commented Sep 9, 2026

View reviewed changes

Merge branch 'master' into fix/AG-58737

1f6d4ef

slvvko marked this pull request as ready for review September 9, 2026 20:52

slvvko requested review from Alex-302 and maximtop September 9, 2026 20:52

AG-58737 Fix README anchor in the Slack activity legend

8d07a69

The link pointed to a nonexistent '#github-stats-cli-app' anchor; the 'Activity points' section carries the 'activity_count' anchor.

maximtop requested changes Sep 10, 2026

View reviewed changes

Comment thread examples/publish-stats.yaml Outdated

Comment thread AGENTS.md Outdated

slvvko added 3 commits September 10, 2026 13:27

AG-58737 Use yarn run publish in docs and examples

6b0b801

AG-58737 Render dist/ as inline code in AGENTS.md

94e641d

AG-58737 Fix AGENTS.md commands table alignment

30efa13

slvvko requested a review from maximtop September 10, 2026 17:28

Merge branch 'master' into fix/AG-58737

76f0449

slvvko mentioned this pull request Sep 11, 2026

AG-58752 Upgrade @slack/web-api to 8.1.1 to drop vulnerable axios #34

Merged

6 tasks done

maximtop approved these changes Sep 11, 2026

View reviewed changes

Comment thread src/prepare-stats/prepare-repo-stat.js Outdated

slvvko added 2 commits September 11, 2026 08:53

Merge branch 'master' into fix/AG-58737

7d92d3f

slvvko requested a review from maximtop September 11, 2026 13:04

slvvko merged commit ac798b1 into master Sep 11, 2026
1 check passed

slvvko deleted the fix/AG-58737 branch September 11, 2026 13:32

Conversation

slvvko commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AG-58737

Problem

Fix

Merge conflict resolution

Dropped npm bin field, renamed build dir bin/ → dist/

Verification

Review fixes (2026-09-10)

Uh oh!

slvvko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slvvko commented Sep 9, 2026

Uh oh!

slvvko commented Sep 9, 2026

Uh oh!

slvvko commented Sep 9, 2026

Uh oh!

slvvko commented Sep 9, 2026

Uh oh!

slvvko commented Sep 9, 2026

Uh oh!

slvvko commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maximtop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maximtop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

slvvko commented Sep 8, 2026 •

edited

Loading

Dropped npm `bin` field, renamed build dir `bin/` → `dist/`

slvvko commented Sep 9, 2026 •

edited

Loading