Add the ability to use local optimization_config by mu-hun · Pull Request #276 · AdguardTeam/FiltersCompiler

mu-hun · 2026-04-24T06:35:34Z

continue from #275

Related: AdguardTeam/FiltersRegistry#1186

…ization' To ensure consistency with other names. Most variable and function names in that file start with 'optimization' without the 'filter' prefix.

mu-hun · 2026-04-24T06:37:03Z

commit singing missed 😅

Add unit tests covering `optimizationConfigLocal` and its members. Introduce `optimizationConfigLocal` namespace with three members: - `setPath(configPath)`: Sets the local config directory path. When set, `getOptimizationPercent` and `getOptimizationConfig` read from local files instead of downloading from remote URLs. - `generate(configPath)`: Downloads `percent.json` and per-filter `stats.json` from remote and saves them to `configPath`, creating `configPath/percent.json` and `configPath/filters/{filterId}/stats.json`. Per-filter file writes run in parallel via `Promise.all`. - `reset()`: Clears the local config path and nulls the in-memory `optimizationPercent` cache. Intended for restoring clean state between tests.

GitHub shorthand installation does not trigger a build step, so dist/ must be present in the branch directly. This is a temporary measure for the PR review period. dist/ should be removed from the branch before merging, and the version should be published to npm instead.

…onStats "Config" was ambiguous — the function returns per-filter stats (hit counts, groups), not configuration. "Stats" matches the source data (stats.json) and the new generateStats() method name.

…izationConfig BREAKING CHANGE: exported name changed. Update all import sites: - before: import { optimizationConfigLocal } from '@adguard/filters-compiler' - after: import { localOptimizationConfig } from '@adguard/filters-compiler' Noun-first naming matches the other module-level vars (localOptimizationConfigPath, optimizationPercent).

download percent.json locally first, enabling users to edit it Before stats are fetched. downloadStatsFromPercentJson() fills in missing stats via two-steps lookup: local file → network download. Local stats.json files are preserved (never overwritten), so user edits to stats directly affect the compiled output. getOptimizationConfig() reads from in-memory cache (populated by downloadStatsFromPercentJson()) when localOptimizationConfigPath is set, instead of re-reading the file on every call.

reset() now takes an explicit configPath instead of reading stored state. getOptimizationStats() simplified: cache hit → return, else download. BREAKING CHANGE: setPath(), getOptimizationPercent() no longer exported; reset() signature changed from reset() to reset(configPath).

mu-hun · 2026-05-20T09:52:00Z

Including dist/ temporarily for testing/review is understandable.

I will perform a force-push to exclude the changes in the dist/ directory. Separating them into a new commit makes the review process easier.

maximtop

I also tested the compiled package with optimization enabled. Platform generation fails on a non-optimized filter because the new code requests filters/10/stats.json and the server returns 404. Existing builder tests do not catch this because they call disableOptimization().

maximtop · 2026-05-20T11:06:39Z

+describe('local optimization config', () => {
+    let tmpDir;
+
+    afterEach(() => {


reset is async but this afterEach does not await or return it. Also, the nested test block declares another const tmpDir, so cleanup still uses the outer tmpDir and leaves the stats test directory behind.

Please make cleanup awaited and avoid shadowing tmpDir.

I applied your comment. Please see the changes.

By the way, should I keep using fs.promises in 'reset()` or switch to synchronous methods?

I mean that the others, like readFile, writeFile, and mkdtemp, are called either synchronously or asynchronously in the existing codebase.

In short: When should I call the node fs module synchronously or asynchronously in this project?

In this PR, I switched to using fs.promises today for consistency (d393a90) because I had been using both.

I applied your comment. Please see the changes.

By the way, should I keep using fs.promises in 'reset()` or switch to synchronous methods?

I mean that the others, like readFile, writeFile, and mkdtemp, are called either synchronously or asynchronously in the existing codebase.

In short: When should I call the node fs module synchronously or asynchronously in this project?

For this PR, fs.promises is the right direction. These helpers are already async and can run during build workflows, so async filesystem calls avoid blocking the event loop and keep the API consistent.

Sync fs is fine for tiny startup/config reads or test setup when it makes tests simpler, but new runtime/build-path code should generally prefer fs.promises.

Without the gate, every uncached filterId called filters/{id}/stats.json, which 404s for non-optimized filters. - Re-introduce optimizableFilterIds Set (lazily loaded from percent.json on first getOptimizationStats call) - Return null for filterIds not listed in percent.json - Restore groups array validation on downloaded stats - Populate optimizableFilterIds in downloadStatsFromPercentJson - Reset optimizableFilterIds to null in reset() - Extract PERCENT_JSON / STATS_JSON / FILTERS_DIR constants - Add JSDoc to localOptimizationConfig and getOptimizationStats

- Move async setup from describe callbacks into beforeAll/afterAll - Remove inner const tmpDir shadowing the outer let tmpDir - Await localOptimizationConfig.reset() in cleanup hooks - Add: getOptimizationStats returns null for unlisted filterId and does not call the stats endpoint

maximtop

Manual review — 13 findings (mostly cosmetic/style). No bugs found, tests pass. All issues are non-blocking.

`global-require` disable was left over from old CommonJS code. No `require()` calls remain in the file. Refs #276 (comment)

Refs #276 (comment)

@type

First @type {Promise<Set<number>>|null} comment was orphaned — not adjacent to any declaration. The correct block preceding `optimizableFilterIdsPromise` remains. Refs #276 (comment)

Refs #276 (comment)

Replace .then() chain with async IIFE for consistency with the rest of the codebase. Refs #276 (comment)

Refs #276 (comment)

Sync fs call inside async mapper. Use async access() with try/catch and move download into catch block to avoid the flag variable. Refs #276 (comment)

Predicate function returns a boolean; action-verb name was misleading. Also fixes stale @param JSDoc reference (getFilterOptimizationConfig → getOptimizationStats). Refs #276 (comment)

Verbose multi-bullet descriptions replaced with brief one-liners per project convention. Refs #276 (comment)

Refs #276 (comment)

"three exported functions" → "four exports"; drop the directory-tree block from localOptimizationConfig — ASCII art renders poorly in IDE tooltips. Refs #276 (comment) Refs #276 (comment)

It has been changed by formatter

#276 (comment)

mu-hun · 2026-06-04T05:33:04Z

~~I'm ready to remove the /dist~~ already deleted at 5495b31 (this PR)

mu-hun · 2026-06-04T07:30:05Z

When will it be merged? Does it need to await the reset reviewer?

Alex-302 · 2026-06-05T08:42:23Z

You need to move the PR to another repository. I'll post about it in our chat.

Clean-up: Rename to begin with 'optimization' instead of 'filterOptim…

48bf377

…ization' To ensure consistency with other names. Most variable and function names in that file start with 'optimization' without the 'filter' prefix.

mu-hun requested a review from Alex-302 April 24, 2026 06:35

mu-hun self-assigned this Apr 24, 2026

mu-hun force-pushed the local_optimization_config branch from a69c0f0 to e54e49d Compare April 24, 2026 06:44

mu-hun added 2 commits April 27, 2026 16:50

mu-hun force-pushed the local_optimization_config branch from e54e49d to 615d6e0 Compare April 27, 2026 07:51

mu-hun added 2 commits May 7, 2026 14:19

fix: load stats file from local if exist

9283da7

upd: /dist

2231ebc

mu-hun mentioned this pull request May 7, 2026

Add the ability to use local optimization_config AdguardTeam/FiltersRegistry#1186

Open

adguard closed this May 12, 2026

adguard deleted the local_optimization_config branch May 12, 2026 10:34

Alex-302 restored the local_optimization_config branch May 13, 2026 07:51

Alex-302 reopened this May 13, 2026

adguard closed this May 14, 2026

adguard deleted the local_optimization_config branch May 14, 2026 11:31

Alex-302 restored the local_optimization_config branch May 15, 2026 17:58

Alex-302 reopened this May 15, 2026

mu-hun added 4 commits May 20, 2026 00:31

refactor(optimization): rename getOptimizationConfig to getOptimizati…

98e6f6b

…onStats "Config" was ambiguous — the function returns per-filter stats (hit counts, groups), not configuration. "Stats" matches the source data (stats.json) and the new generateStats() method name.

upd: dist/

3e24d93

mu-hun force-pushed the local_optimization_config branch from 238440f to 3e24d93 Compare May 20, 2026 09:52

maximtop reviewed May 20, 2026

View reviewed changes

mu-hun added 3 commits May 21, 2026 18:11

docs(changelog): add [Unreleased] entry for localOptimizationConfig

2df6b52

mu-hun marked this pull request as ready for review May 27, 2026 12:16

maximtop reviewed May 29, 2026

View reviewed changes

maximtop requested changes May 29, 2026

View reviewed changes

mu-hun added 11 commits May 29, 2026 23:02

refactor: remove dead eslint-disable directive

dd9e4e0

`global-require` disable was left over from old CommonJS code. No `require()` calls remain in the file. Refs #276 (comment)

docs: add JSDoc to downloadOptimizationStats

96ade7b

Refs #276 (comment)

docs: remove orphaned duplicate JSDoc block

6498796

First @type {Promise<Set<number>>|null} comment was orphaned — not adjacent to any declaration. The correct block preceding `optimizableFilterIdsPromise` remains. Refs #276 (comment)

docs: add JSDoc to getOptimizableFilterIds

280e016

Refs #276 (comment)

refactor: rewrite getOptimizableFilterIds with async/await

2a34333

Replace .then() chain with async IIFE for consistency with the rest of the codebase. Refs #276 (comment)

docs: add description to assertValidStats JSDoc

c9ba17b

Refs #276 (comment)

refactor: replace fs.existsSync with fs.promises.access

370d751

Sync fs call inside async mapper. Use async access() with try/catch and move download into catch block to avoid the flag variable. Refs #276 (comment)

refactor: rename skipRuleWithOptimization to shouldSkipRule

892bafa

Predicate function returns a boolean; action-verb name was misleading. Also fixes stale @param JSDoc reference (getFilterOptimizationConfig → getOptimizationStats). Refs #276 (comment)

docs: shorten Unreleased CHANGELOG entries

b93a74e

Verbose multi-bullet descriptions replaced with brief one-liners per project convention. Refs #276 (comment)

docs: add localOptimizationConfig to index.js tree comment

67f53e5

Refs #276 (comment)

docs(index.d.ts): fix export count + remove ASCII layout from JSDoc

6c1aa71

"three exported functions" → "four exports"; drop the directory-tree block from localOptimizationConfig — ASCII art renders poorly in IDE tooltips. Refs #276 (comment) Refs #276 (comment)

mu-hun requested a review from maximtop May 29, 2026 14:33

mu-hun added 3 commits May 29, 2026 23:37

fix: revert the useless CHANGELOG changes to fit master

5c60219

It has been changed by formatter

chore: formatting

ea05d33

clean: directly uses 'fs/promises'

0774207

maximtop reviewed Jun 3, 2026

View reviewed changes

Comment thread src/main/optimization.js Outdated

maximtop reviewed Jun 3, 2026

View reviewed changes

Comment thread src/index.d.ts Outdated

maximtop requested changes Jun 3, 2026

View reviewed changes

mu-hun added 2 commits June 4, 2026 14:31

fix: do not use async in the wrapper

8155c7f

#276 (comment)

fix: don't write block comment in one line

c277bb9

#276 (comment)

mu-hun requested a review from maximtop June 4, 2026 05:32

maximtop approved these changes Jun 4, 2026

View reviewed changes

adguard closed this Jun 5, 2026

adguard deleted the local_optimization_config branch June 5, 2026 10:58

Conversation

mu-hun commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mu-hun commented Apr 24, 2026

Uh oh!

mu-hun commented May 20, 2026

Uh oh!

maximtop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maximtop May 20, 2026

Choose a reason for hiding this comment

Uh oh!

mu-hun May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mu-hun May 21, 2026

Choose a reason for hiding this comment

Uh oh!

maximtop May 22, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mu-hun commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mu-hun commented Jun 4, 2026

Uh oh!

Alex-302 commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mu-hun commented Apr 24, 2026 •

edited

Loading

mu-hun May 21, 2026 •

edited

Loading

mu-hun commented Jun 4, 2026 •

edited

Loading