zombor/pergamum
1
0
Fork 0
Code Issues Pull requests 3 Projects Releases Packages 2 Wiki Activity Actions

feat(bulk-scan): cost guardrails (budget + dry-run estimate) + progress/metrics (bookshelf-cs2zl.4) #1475

Open
zombor wants to merge 2 commits from bd-bookshelf-cs2zl.4 into main
pull from: bd-bookshelf-cs2zl.4
merge into: zombor:main
Conversation 9 Commits 2 Files changed 19 +1161 -21
zombor commented 2026-09-07 02:18:02 +00:00
Owner
Copy link

Summary

Final vertical slice of the bulk scan-file-for-metadata epic (cs2zl.1-.3): cost guardrails + observability.

  • Budget cap: new scan_file.max_books_per_run setting (Settings -> Metadata -> LLM Scan Settings, default 500). The by-ids toolbar trigger hard-rejects a selection that exceeds the budget. The by-filter/whole-library trigger enforces the budget as a running total inside BulkByFilterWorkflow across ContinueAsNew epochs (scoped to the scan-file op only), since the exact match count isn't cheaply known up front at that scale.
  • Dry-run estimate: the existing "Scan All Files for Metadata" confirmation dialog now shows a rough time estimate (~N sec/min/hr) alongside the book count.
  • Metrics: new internal/books.ScanFileMetrics — scan_file_bulk_applied_total, _review_queued_total, _skipped_total, _apply_failed_total, _review_failed_total.
  • Progress: the Settings -> Workflows "Running" section progress mechanism is already generic (parent/child workflow relationship), so scan-file bulk runs show progress with no additional code.

Replay-safety / workflow versioning note

This PR changes the command sequence of ScanFileBookWorkflow (adds a RecordSkip activity call on the below-confidence-threshold branch) and adds new carry-forward fields to BulkByFilterInput/BulkScanFileByIDsInput/ScanFileBookInput. go-workflows in this codebase has no gowf.Version primitive, so this follows the established replay-safe input-flag-gate pattern already used by ScanFileBookInput.ReviewMode and BookdropAutoFetchInput.ResolveOnTerminalFailure:

  • ScanFileMaxBooks/ScanFileProcessed (BulkByFilterInput) default to 0 for any serialized input that predates this change, preserving the original unbounded/no-early-stop behavior for in-flight and other-op instances.
  • EmitSkipMetric (ScanFileBookInput) defaults to false for any serialized input that predates this change, so an in-flight instance replays its original (no RecordSkip call) path; only brand-new sub-workflow instances (created after this deploy) set it true and take the new path.

Test plan

  • go test ./internal/settings/... ./internal/wfengine/... ./internal/books/... — new budget-cap and metrics tests, including workflow-tester coverage of the budget-truncation and exact-boundary cases.
  • make lint — clean.
  • make coverage — 100% on internal/.
  • npm run coverage — 100% JS (new/updated Vitest specs for scan_file_settings_controller.js and library_kebab_menu_controller.js).
  • Rendered screenshot of the new "Max books per run" field in Settings -> Metadata -> LLM Scan Settings (posted below).

Closes bead bookshelf-cs2zl.4 on merge.

## Summary Final vertical slice of the bulk scan-file-for-metadata epic (cs2zl.1-.3): cost guardrails + observability. - **Budget cap**: new `scan_file.max_books_per_run` setting (Settings -> Metadata -> LLM Scan Settings, default 500). The by-ids toolbar trigger hard-rejects a selection that exceeds the budget. The by-filter/whole-library trigger enforces the budget as a running total inside `BulkByFilterWorkflow` across `ContinueAsNew` epochs (scoped to the scan-file op only), since the exact match count isn't cheaply known up front at that scale. - **Dry-run estimate**: the existing "Scan All Files for Metadata" confirmation dialog now shows a rough time estimate (`~N sec/min/hr`) alongside the book count. - **Metrics**: new `internal/books.ScanFileMetrics` — `scan_file_bulk_applied_total`, `_review_queued_total`, `_skipped_total`, `_apply_failed_total`, `_review_failed_total`. - **Progress**: the Settings -> Workflows "Running" section progress mechanism is already generic (parent/child workflow relationship), so scan-file bulk runs show progress with no additional code. ## Replay-safety / workflow versioning note This PR changes the command sequence of `ScanFileBookWorkflow` (adds a `RecordSkip` activity call on the below-confidence-threshold branch) and adds new carry-forward fields to `BulkByFilterInput`/`BulkScanFileByIDsInput`/`ScanFileBookInput`. go-workflows in this codebase has no `gowf.Version` primitive, so this follows the established **replay-safe input-flag-gate** pattern already used by `ScanFileBookInput.ReviewMode` and `BookdropAutoFetchInput.ResolveOnTerminalFailure`: - `ScanFileMaxBooks`/`ScanFileProcessed` (BulkByFilterInput) default to `0` for any serialized input that predates this change, preserving the original unbounded/no-early-stop behavior for in-flight and other-op instances. - `EmitSkipMetric` (ScanFileBookInput) defaults to `false` for any serialized input that predates this change, so an in-flight instance replays its original (no `RecordSkip` call) path; only brand-new sub-workflow instances (created after this deploy) set it `true` and take the new path. ## Test plan - [x] `go test ./internal/settings/... ./internal/wfengine/... ./internal/books/...` — new budget-cap and metrics tests, including workflow-tester coverage of the budget-truncation and exact-boundary cases. - [x] `make lint` — clean. - [x] `make coverage` — 100% on `internal/`. - [x] `npm run coverage` — 100% JS (new/updated Vitest specs for `scan_file_settings_controller.js` and `library_kebab_menu_controller.js`). - [x] Rendered screenshot of the new "Max books per run" field in Settings -> Metadata -> LLM Scan Settings (posted below). Closes bead bookshelf-cs2zl.4 on merge.
llm_scan_settings.png
68 KiB
llm_scan_settings.png
feat(bulk-scan): cost guardrails (budget cap + dry-run estimate) + metrics (bookshelf-cs2zl.4)
All checks were successful
/ E2E API (pull_request) Successful in 1m18s
Details
/ Test Race (pull_request) Successful in 1m49s
Details
/ JS Unit Tests (pull_request) Successful in 50s
Details
/ Lint (pull_request) Successful in 2m28s
Details
/ Coverage (pull_request) Successful in 2m59s
Details
/ Integration (pull_request) Successful in 3m2s
Details
/ E2E Browser (pull_request) Successful in 5m4s
Details
ee57ca46d2
 
Adds the final guardrail layer to the bulk scan-file-for-metadata feature
(cs2zl.1-.3): a configurable per-run budget cap so a bulk LLM file-scan
cannot silently run unbounded across a 250k-book library, a rough dry-run
time estimate in the by-filter/whole-library confirmation dialog, and
Prometheus counters for applied/review-queued/skipped/failed outcomes.

Budget cap:
- New setting scan_file.max_books_per_run (Settings -> Metadata -> LLM Scan
  Settings), default 500, persisted via app_settings.
- By-ids trigger (toolbar selection): hard reject with a validation error
  when the selected book count exceeds the configured budget.
- By-filter/whole-library trigger: since the exact match count is not known
  cheaply up front (that's the reason this path paginates), the budget is
  enforced as a running total inside BulkByFilterWorkflow across
  ContinueAsNew epochs, scoped to the scan-file op only. The new
  ScanFileMaxBooks/ScanFileProcessed fields default to zero for any
  in-flight instance (and are ignored by every other op), so this does not
  change the command sequence for existing running instances or other bulk
  ops -- the same replay-safe input-flag-gate technique already used by
  ScanFileBookInput.ReviewMode.

Dry-run estimate:
- The existing "Scan All Files for Metadata" library-kebab confirmation
  dialog now shows a rough time estimate (~sec/min/hr) alongside the book
  count, computed client-side from a documented per-book constant.

Metrics + progress:
- New internal/books.ScanFileMetrics: scan_file_bulk_applied_total,
  _review_queued_total, _skipped_total, _apply_failed_total,
  _review_failed_total. Applied/review-queued/failed counters are
  incremented from inside the existing ApplyScanResult/StoreScanFileReview
  activities (activity internals -- no workflow command-sequence change).
  The skipped counter is recorded via a new RecordSkip activity, gated by a
  new EmitSkipMetric input flag (same replay-safe pattern as ReviewMode) so
  only brand-new sub-workflow instances take the new path.
- Progress in the Settings -> Workflows "Running" section is already
  generic (derived from parent/child workflow-instance relationships), so
  BulkScanFileByIDsWorkflow / BulkFilterOpScanFile fan-outs show progress
  with no additional code.

Test plan:
- go test ./internal/settings/... ./internal/wfengine/... ./internal/books/...
- make lint / make coverage (100% on internal/)
- npm run coverage (100% JS)
- Screenshot of the new "Max books per run" field posted to the PR.

Closes bead bookshelf-cs2zl.4 on merge.
zombor commented 2026-09-07 02:18:10 +00:00
Author
Owner
Copy link

Screenshot: new Max books per run budget-cap field in Settings → Metadata → LLM Scan Settings.

Screenshot: new **Max books per run** budget-cap field in Settings → Metadata → LLM Scan Settings.
zombor commented 2026-09-07 02:18:36 +00:00
Author
Owner
Copy link

Screenshot: new Max books per run budget-cap field in Settings -> Metadata -> LLM Scan Settings.

llm_scan_settings.png

Screenshot: new **Max books per run** budget-cap field in Settings -> Metadata -> LLM Scan Settings. ![llm_scan_settings.png](/attachments/c602d8b9-d9ba-4d4b-bc8c-a7c80ce7f282)
zombor commented 2026-09-07 02:27:01 +00:00
Author
Owner
Copy link

UI Review (rendered screenshot)

Reviewed the attached screenshot of the LLM Scan Settings section plus the templates/pages/settings_shell.html diff.

[MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a .form-hint, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (.form-hint is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation.

What's good:

  • Reuses the canonical .metadata-field / .metadata-field-label / .metadata-field-input / .form-hint classes exactly — no bespoke styling.
  • No inline style= anywhere in the diff (grep confirmed).
  • Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches.
  • "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips.
  • Helper copy is clear and voice-consistent with the section description and other hint text on the page.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## UI Review (rendered screenshot) Reviewed the attached screenshot of the LLM Scan Settings section plus the `templates/pages/settings_shell.html` diff. [MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a `.form-hint`, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (`.form-hint` is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation. What's good: - Reuses the canonical `.metadata-field` / `.metadata-field-label` / `.metadata-field-input` / `.form-hint` classes exactly — no bespoke styling. - No inline `style=` anywhere in the diff (grep confirmed). - Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches. - "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips. - Helper copy is clear and voice-consistent with the section description and other hint text on the page. REVIEW VERDICT: 0 blocker, 0 major, 1 minor
zombor commented 2026-09-07 02:27:12 +00:00
Author
Owner
Copy link

UI Review (rendered screenshot)

Reviewed the attached screenshot of the LLM Scan Settings section plus the templates/pages/settings_shell.html diff.

[MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a .form-hint, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (.form-hint is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation.

What's good:

  • Reuses the canonical .metadata-field / .metadata-field-label / .metadata-field-input / .form-hint classes exactly — no bespoke styling.
  • No inline style= anywhere in the diff (grep confirmed).
  • Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches.
  • "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips.
  • Helper copy is clear and voice-consistent with the section description and other hint text on the page.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## UI Review (rendered screenshot) Reviewed the attached screenshot of the LLM Scan Settings section plus the `templates/pages/settings_shell.html` diff. [MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a `.form-hint`, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (`.form-hint` is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation. What's good: - Reuses the canonical `.metadata-field` / `.metadata-field-label` / `.metadata-field-input` / `.form-hint` classes exactly — no bespoke styling. - No inline `style=` anywhere in the diff (grep confirmed). - Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches. - "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips. - Helper copy is clear and voice-consistent with the section description and other hint text on the page. REVIEW VERDICT: 0 blocker, 0 major, 1 minor
zombor commented 2026-09-07 02:27:20 +00:00
Author
Owner
Copy link

UI Review (rendered screenshot)

Reviewed the attached screenshot of the LLM Scan Settings section plus the templates/pages/settings_shell.html diff.

[MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a .form-hint, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (.form-hint is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation.

What's good:

  • Reuses the canonical .metadata-field / .metadata-field-label / .metadata-field-input / .form-hint classes exactly — no bespoke styling.
  • No inline style= anywhere in the diff (grep confirmed).
  • Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches.
  • "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips.
  • Helper copy is clear and voice-consistent with the section description and other hint text on the page.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## UI Review (rendered screenshot) Reviewed the attached screenshot of the LLM Scan Settings section plus the `templates/pages/settings_shell.html` diff. [MINOR] templates/pages/settings_shell.html:730 — new field is the only one of the four in this section with a `.form-hint`, slightly denser than its three siblings (Ebook/Comic threshold, review-mode checkbox have none). Not a foreign pattern (`.form-hint` is used elsewhere on this same page, e.g. line 614's token-budget field with near-identical shape), so this is a nit, not a defect — consider adding brief hints to the sibling threshold fields too for consistency, or leave as-is since the budget-cap semantics genuinely need more explanation. What's good: - Reuses the canonical `.metadata-field` / `.metadata-field-label` / `.metadata-field-input` / `.form-hint` classes exactly — no bespoke styling. - No inline `style=` anywhere in the diff (grep confirmed). - Spacing/vertical rhythm in the screenshot matches the sibling Ebook/Comic threshold fields and the review-mode checkbox above it (~65px per field block); input width matches; label size/weight matches. - "Save LLM Scan Settings" button and "View pending scan-file reviews →" link remain correctly grouped below the new field, nothing overlaps or clips. - Helper copy is clear and voice-consistent with the section description and other hint text on the page. REVIEW VERDICT: 0 blocker, 0 major, 1 minor
zombor commented 2026-09-07 02:27:42 +00:00
Author
Owner
Copy link

Security Review — PR #1475 (bd-bookshelf-cs2zl.4)

Reviewed the full diff (wfengine bulk_by_filter/bulk_scan_file workflows, app wiring, settings, metrics, JS/templates).

Findings: none rising to BLOCKER/MAJOR.

Verification performed:

  1. Budget cap is genuine and server-enforced.

    • StartBulkScanFileByIDs (internal/app/build_wf_triggers.go) hard-rejects (422 via middleware.ErrValidation) a by-ids request whose len(bookIDs) > maxBooks, where maxBooks comes from settings.LoadScanFileSettings / DefaultScanFileMaxBooksPerRun — never from the request body.
    • The by-filter/whole-library path enforces the cap as a running total (ScanFileMaxBooks/ScanFileProcessed) carried across ContinueAsNew epochs via scanFileBudgetTruncate, which truncates the batch and stops the workflow (no further ContinueAsNew) once the budget is exhausted — confirmed by real multi-epoch tests in bulk_by_filter_workflow_test.go ("scan-file op with a budget cap" / "...landing exactly on a batch boundary") that assert the workflow stops without spawning a further epoch.
    • MaxScanFileMaxBooksPerRun (100000) bounds the admin-configurable value itself, preventing an operator from disabling the guardrail via an absurd setting.
    • ScanFileMaxBooks/ScanFileProcessed are new BulkByFilterInput fields that default to 0 (unlimited) on unmarshal for pre-existing in-flight instances — correct replay-safety per the project's workflow-versioning convention (no command-sequence change, only a data value), matching the existing ReviewMode/ResolveOnTerminalFailure pattern.

  2. Multi-user / authz — unaffected by this PR. POST /books/bulk/scan-file, POST /books/bulk/filter/scan-file route registration (internal/books/routes.go) and by-filter library/user scoping (BulkByFilterFilter.UserID, UserLibraryIDs) are untouched by this diff; the budget fields are purely additive and do not widen any existing scope.

  3. Settings authz — GET/PUT /settings/scan-file-settings remain behind adminRequired (internal/settings/routes.go, unchanged). New MaxBooksPerRun field is validated server-side (validateMaxBooksPerRun: range [0, 100000], tested for negative and over-max in scan_file_settings_test.go) before persisting to app_settings.

  4. Metrics — internal/books/scan_file_metrics.go counters (scan_file_bulk_applied_total, ..._review_queued_total, ..._skipped_total, ..._apply_failed_total, ..._review_failed_total) carry no labels at all — no book IDs, filenames, titles, or user identifiers as label values, so no PII leak and no cardinality risk.

  5. Template/JS — no inline style= introduced. The new _scanFileEstimate dry-run message is built from a Stimulus-supplied numeric bookCountValue (not free-text/HTML), and the confirm dialog message is plain text via AppDialog.confirm, not an HTML sink — no injection vector. New settings input (#scan-file-max-books-per-run) is a plain numeric <input type="number"> bound via Stimulus target, value rendered through Go template's default (HTML-escaped) {{.MetaScanFileSettings.MaxBooksPerRun}}.

  6. Logging — SaveScanFileSettingsHandler logs only numeric threshold/bool/int values (no secrets/PII); consistent with existing logging in that handler.

Minor process notes (non-blocking, informational only):

  • internal/wfengine/export_test.go additions (NewBulkScanFileActivitiesWithMetrics, BulkScanFileMetricHooksExport) are new white-box test exports, but internal/wfengine carries a standing, documented exception for this pattern (func-field DI + MySQL-only constructor makes black-box conversion impractical) — consistent with prior PRs in this package.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security Review — PR #1475 (bd-bookshelf-cs2zl.4) Reviewed the full diff (wfengine bulk_by_filter/bulk_scan_file workflows, app wiring, settings, metrics, JS/templates). **Findings: none rising to BLOCKER/MAJOR.** Verification performed: 1. **Budget cap is genuine and server-enforced.** - `StartBulkScanFileByIDs` (internal/app/build_wf_triggers.go) hard-rejects (422 via `middleware.ErrValidation`) a by-ids request whose `len(bookIDs) > maxBooks`, where `maxBooks` comes from `settings.LoadScanFileSettings` / `DefaultScanFileMaxBooksPerRun` — never from the request body. - The by-filter/whole-library path enforces the cap as a running total (`ScanFileMaxBooks`/`ScanFileProcessed`) carried across `ContinueAsNew` epochs via `scanFileBudgetTruncate`, which truncates the batch and stops the workflow (no further `ContinueAsNew`) once the budget is exhausted — confirmed by real multi-epoch tests in `bulk_by_filter_workflow_test.go` ("scan-file op with a budget cap" / "...landing exactly on a batch boundary") that assert the workflow stops without spawning a further epoch. - `MaxScanFileMaxBooksPerRun` (100000) bounds the admin-configurable value itself, preventing an operator from disabling the guardrail via an absurd setting. - `ScanFileMaxBooks`/`ScanFileProcessed` are new `BulkByFilterInput` fields that default to 0 (unlimited) on unmarshal for pre-existing in-flight instances — correct replay-safety per the project's workflow-versioning convention (no command-sequence change, only a data value), matching the existing `ReviewMode`/`ResolveOnTerminalFailure` pattern. 2. **Multi-user / authz** — unaffected by this PR. `POST /books/bulk/scan-file`, `POST /books/bulk/filter/scan-file` route registration (internal/books/routes.go) and by-filter library/user scoping (`BulkByFilterFilter.UserID`, `UserLibraryIDs`) are untouched by this diff; the budget fields are purely additive and do not widen any existing scope. 3. **Settings authz** — `GET/PUT /settings/scan-file-settings` remain behind `adminRequired` (internal/settings/routes.go, unchanged). New `MaxBooksPerRun` field is validated server-side (`validateMaxBooksPerRun`: range `[0, 100000]`, tested for negative and over-max in scan_file_settings_test.go) before persisting to `app_settings`. 4. **Metrics** — `internal/books/scan_file_metrics.go` counters (`scan_file_bulk_applied_total`, `..._review_queued_total`, `..._skipped_total`, `..._apply_failed_total`, `..._review_failed_total`) carry **no labels at all** — no book IDs, filenames, titles, or user identifiers as label values, so no PII leak and no cardinality risk. 5. **Template/JS** — no inline `style=` introduced. The new `_scanFileEstimate` dry-run message is built from a Stimulus-supplied numeric `bookCountValue` (not free-text/HTML), and the confirm dialog message is plain text via `AppDialog.confirm`, not an HTML sink — no injection vector. New settings input (`#scan-file-max-books-per-run`) is a plain numeric `<input type="number">` bound via Stimulus target, value rendered through Go template's default (HTML-escaped) `{{.MetaScanFileSettings.MaxBooksPerRun}}`. 6. **Logging** — `SaveScanFileSettingsHandler` logs only numeric threshold/bool/int values (no secrets/PII); consistent with existing logging in that handler. Minor process notes (non-blocking, informational only): - `internal/wfengine/export_test.go` additions (`NewBulkScanFileActivitiesWithMetrics`, `BulkScanFileMetricHooksExport`) are new white-box test exports, but `internal/wfengine` carries a standing, documented exception for this pattern (func-field DI + MySQL-only constructor makes black-box conversion impractical) — consistent with prior PRs in this package. REVIEW VERDICT: 0 blocker, 0 major, 0 minor
zombor commented 2026-09-07 02:27:46 +00:00
Author
Owner
Copy link

Security Review — PR #1475 (bd-bookshelf-cs2zl.4)

Reviewed the full diff (wfengine bulk_by_filter/bulk_scan_file workflows, app wiring, settings, metrics, JS/templates).

Findings: none rising to BLOCKER/MAJOR.

Verification performed:

  1. Budget cap is genuine and server-enforced.

    • StartBulkScanFileByIDs (internal/app/build_wf_triggers.go) hard-rejects (422 via middleware.ErrValidation) a by-ids request whose len(bookIDs) > maxBooks, where maxBooks comes from settings.LoadScanFileSettings / DefaultScanFileMaxBooksPerRun — never from the request body.
    • The by-filter/whole-library path enforces the cap as a running total (ScanFileMaxBooks/ScanFileProcessed) carried across ContinueAsNew epochs via scanFileBudgetTruncate, which truncates the batch and stops the workflow (no further ContinueAsNew) once the budget is exhausted — confirmed by real multi-epoch tests in bulk_by_filter_workflow_test.go ("scan-file op with a budget cap" / "...landing exactly on a batch boundary") that assert the workflow stops without spawning a further epoch.
    • MaxScanFileMaxBooksPerRun (100000) bounds the admin-configurable value itself, preventing an operator from disabling the guardrail via an absurd setting.
    • ScanFileMaxBooks/ScanFileProcessed are new BulkByFilterInput fields that default to 0 (unlimited) on unmarshal for pre-existing in-flight instances — correct replay-safety per the project's workflow-versioning convention (no command-sequence change, only a data value), matching the existing ReviewMode/ResolveOnTerminalFailure pattern.

  2. Multi-user / authz — unaffected by this PR. POST /books/bulk/scan-file, POST /books/bulk/filter/scan-file route registration (internal/books/routes.go) and by-filter library/user scoping (BulkByFilterFilter.UserID, UserLibraryIDs) are untouched by this diff; the budget fields are purely additive and do not widen any existing scope.

  3. Settings authz — GET/PUT /settings/scan-file-settings remain behind adminRequired (internal/settings/routes.go, unchanged). New MaxBooksPerRun field is validated server-side (validateMaxBooksPerRun: range [0, 100000], tested for negative and over-max in scan_file_settings_test.go) before persisting to app_settings.

  4. Metrics — internal/books/scan_file_metrics.go counters (scan_file_bulk_applied_total, ..._review_queued_total, ..._skipped_total, ..._apply_failed_total, ..._review_failed_total) carry no labels at all — no book IDs, filenames, titles, or user identifiers as label values, so no PII leak and no cardinality risk.

  5. Template/JS — no inline style= introduced. The new _scanFileEstimate dry-run message is built from a Stimulus-supplied numeric bookCountValue (not free-text/HTML), and the confirm dialog message is plain text via AppDialog.confirm, not an HTML sink — no injection vector. New settings input (#scan-file-max-books-per-run) is a plain numeric <input type="number"> bound via Stimulus target, value rendered through Go template's default (HTML-escaped) {{.MetaScanFileSettings.MaxBooksPerRun}}.

  6. Logging — SaveScanFileSettingsHandler logs only numeric threshold/bool/int values (no secrets/PII); consistent with existing logging in that handler.

Minor process notes (non-blocking, informational only):

  • internal/wfengine/export_test.go additions (NewBulkScanFileActivitiesWithMetrics, BulkScanFileMetricHooksExport) are new white-box test exports, but internal/wfengine carries a standing, documented exception for this pattern (func-field DI + MySQL-only constructor makes black-box conversion impractical) — consistent with prior PRs in this package.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security Review — PR #1475 (bd-bookshelf-cs2zl.4) Reviewed the full diff (wfengine bulk_by_filter/bulk_scan_file workflows, app wiring, settings, metrics, JS/templates). **Findings: none rising to BLOCKER/MAJOR.** Verification performed: 1. **Budget cap is genuine and server-enforced.** - `StartBulkScanFileByIDs` (internal/app/build_wf_triggers.go) hard-rejects (422 via `middleware.ErrValidation`) a by-ids request whose `len(bookIDs) > maxBooks`, where `maxBooks` comes from `settings.LoadScanFileSettings` / `DefaultScanFileMaxBooksPerRun` — never from the request body. - The by-filter/whole-library path enforces the cap as a running total (`ScanFileMaxBooks`/`ScanFileProcessed`) carried across `ContinueAsNew` epochs via `scanFileBudgetTruncate`, which truncates the batch and stops the workflow (no further `ContinueAsNew`) once the budget is exhausted — confirmed by real multi-epoch tests in `bulk_by_filter_workflow_test.go` ("scan-file op with a budget cap" / "...landing exactly on a batch boundary") that assert the workflow stops without spawning a further epoch. - `MaxScanFileMaxBooksPerRun` (100000) bounds the admin-configurable value itself, preventing an operator from disabling the guardrail via an absurd setting. - `ScanFileMaxBooks`/`ScanFileProcessed` are new `BulkByFilterInput` fields that default to 0 (unlimited) on unmarshal for pre-existing in-flight instances — correct replay-safety per the project's workflow-versioning convention (no command-sequence change, only a data value), matching the existing `ReviewMode`/`ResolveOnTerminalFailure` pattern. 2. **Multi-user / authz** — unaffected by this PR. `POST /books/bulk/scan-file`, `POST /books/bulk/filter/scan-file` route registration (internal/books/routes.go) and by-filter library/user scoping (`BulkByFilterFilter.UserID`, `UserLibraryIDs`) are untouched by this diff; the budget fields are purely additive and do not widen any existing scope. 3. **Settings authz** — `GET/PUT /settings/scan-file-settings` remain behind `adminRequired` (internal/settings/routes.go, unchanged). New `MaxBooksPerRun` field is validated server-side (`validateMaxBooksPerRun`: range `[0, 100000]`, tested for negative and over-max in scan_file_settings_test.go) before persisting to `app_settings`. 4. **Metrics** — `internal/books/scan_file_metrics.go` counters (`scan_file_bulk_applied_total`, `..._review_queued_total`, `..._skipped_total`, `..._apply_failed_total`, `..._review_failed_total`) carry **no labels at all** — no book IDs, filenames, titles, or user identifiers as label values, so no PII leak and no cardinality risk. 5. **Template/JS** — no inline `style=` introduced. The new `_scanFileEstimate` dry-run message is built from a Stimulus-supplied numeric `bookCountValue` (not free-text/HTML), and the confirm dialog message is plain text via `AppDialog.confirm`, not an HTML sink — no injection vector. New settings input (`#scan-file-max-books-per-run`) is a plain numeric `<input type="number">` bound via Stimulus target, value rendered through Go template's default (HTML-escaped) `{{.MetaScanFileSettings.MaxBooksPerRun}}`. 6. **Logging** — `SaveScanFileSettingsHandler` logs only numeric threshold/bool/int values (no secrets/PII); consistent with existing logging in that handler. Minor process notes (non-blocking, informational only): - `internal/wfengine/export_test.go` additions (`NewBulkScanFileActivitiesWithMetrics`, `BulkScanFileMetricHooksExport`) are new white-box test exports, but `internal/wfengine` carries a standing, documented exception for this pattern (func-field DI + MySQL-only constructor makes black-box conversion impractical) — consistent with prior PRs in this package. REVIEW VERDICT: 0 blocker, 0 major, 0 minor
zombor commented 2026-09-07 02:29:56 +00:00
Author
Owner
Copy link

Code Review — PR #1475 (bd-bookshelf-cs2zl.4)

Reviewed the diff only (no re-run of make test/coverage/e2e).

Workflow replay-safety (highest priority) — PASS

  1. BulkByFilterInput.ScanFileMaxBooks/ScanFileProcessed (internal/wfengine/bulk_by_filter_workflow.go:110-121): scanFileBudgetTruncate (line ~262) is gated on input.Op != BulkFilterOpScanFile || input.ScanFileMaxBooks <= 0 — for every non-scan-file op, and for pre-deploy in-flight scan-file instances whose recorded input predates the field (unmarshals to 0), it returns bookIDs unchanged and exhausted=false, so the emitted command sequence (ExecuteActivity/fan-out/ContinueAsNew) is byte-identical to before. Confirmed correctly gated — not a hazard.
  2. ScanFileBookInput.EmitSkipMetric (internal/wfengine/bulk_scan_file_workflow.go:80-88): the new RecordSkip ExecuteActivity (line ~191) is gated on input.EmitSkipMetric, which defaults to false on unmarshal for any in-flight sub-workflow instance recorded before this field existed — replay-safe. New parent-workflow epochs setting it true unconditionally for new sub-workflow instances does not affect the parent's own replay determinism (only sub-workflow input payload content changes, not the parent's command type/order). RecordSkip is a method on the already-registered BulkScanFileActivities struct, registered automatically via ws.registerActivity(scanActs) reflection (worker_set.go:246) — confirmed no missing-activity-registration bug, and module_domains_test.go's real-engine test still exercises that registration path.

Budget cap correctness — PASS

  • By-ids trigger (internal/app/build_wf_triggers.go:298-311) hard-rejects when len(bookIDs) > maxBooks, wrapping middleware.ErrValidation (→ 400).
  • By-filter/whole-library enforces the running total across ContinueAsNew epochs via ScanFileProcessed carried forward (bulk_by_filter_workflow.go:266-286); boundary-exact and mid-epoch-truncation cases are both covered by non-vacuous multi-epoch tests (bulk_by_filter_workflow_test.go:1669, :1749).
  • Default 500 (DefaultScanFileMaxBooksPerRun), 0 = default via EffectiveMaxBooksPerRun; settings round-trip and validation ([0, 100000]) covered in scan_file_settings_test.go.

Metrics — PASS

registerScanFileCounter (internal/books/scan_file_metrics.go) returns the existing collector on AlreadyRegisteredError, safe for repeated app.New calls; non-vacuous counter-value tests via reg.Gather().

Findings

[MAJOR] internal/app/build_wf_triggers.go:298-311 — by-ids budget-reject guardrail has zero test coverage
internal/app is (by project convention) untested at the unit level and relies on e2e tests to prove correctness ("pure wiring; no unit tests by design — correctness verified by e2e"). This PR adds real conditional business logic here — the per-run budget hard-reject for StartBulkScanFileByIDs — but the diff adds no e2e spec (browser or API) exercising it, and none of the existing scan-file e2e journeys (journey_bulk_scan_file_test.go, journey_scan_file_reviews_test.go, journey_library_scan_file_test.go) were touched. This is the primary cost-guardrail the bead exists to add, and it is currently unverified by any automated test — a typo or logic inversion (e.g. < vs >) here would ship silently. Add at least one e2e assertion that POSTing a by-ids selection larger than the configured max_books_per_run returns 400/validation-error and does not start the workflow.

Minor observations (non-blocking)

[MINOR] static/js/controllers/library_kebab_menu_controller.js:398-409 — _scanFileEstimate uses a hardcoded 20 sec/book constant with no relation to any measured or server-configured value. It's documented as "rough"/"dry-run preview," which is honest, but if the LLM per-book latency changes materially the estimate will silently drift out of date with no test or config tying it to reality. Consider a code comment pointer or follow-up bead to revisit if provider latency changes significantly.

[MINOR] internal/settings/scan_file_settings.go:210 — ErrInvalidMaxBooksPerRun is built with fmt.Errorf (no %w) as a sentinel var, unlike its sibling ErrInvalidThreshold which uses errors.New. Cosmetic only (both work identically as comparison sentinels since there's no wrapped verb), but slightly inconsistent style — prefer errors.New for a pure sentinel.

REVIEW VERDICT: 0 blocker, 1 major, 2 minor

## Code Review — PR #1475 (bd-bookshelf-cs2zl.4) Reviewed the diff only (no re-run of make test/coverage/e2e). ### Workflow replay-safety (highest priority) — PASS 1. `BulkByFilterInput.ScanFileMaxBooks`/`ScanFileProcessed` (internal/wfengine/bulk_by_filter_workflow.go:110-121): `scanFileBudgetTruncate` (line ~262) is gated on `input.Op != BulkFilterOpScanFile || input.ScanFileMaxBooks <= 0` — for every non-scan-file op, and for pre-deploy in-flight scan-file instances whose recorded input predates the field (unmarshals to 0), it returns `bookIDs` unchanged and `exhausted=false`, so the emitted command sequence (`ExecuteActivity`/fan-out/`ContinueAsNew`) is byte-identical to before. Confirmed correctly gated — not a hazard. 2. `ScanFileBookInput.EmitSkipMetric` (internal/wfengine/bulk_scan_file_workflow.go:80-88): the new `RecordSkip` `ExecuteActivity` (line ~191) is gated on `input.EmitSkipMetric`, which defaults to `false` on unmarshal for any in-flight sub-workflow instance recorded before this field existed — replay-safe. New parent-workflow epochs setting it `true` unconditionally for new *sub-workflow* instances does not affect the parent's own replay determinism (only sub-workflow input payload content changes, not the parent's command type/order). `RecordSkip` is a method on the already-registered `BulkScanFileActivities` struct, registered automatically via `ws.registerActivity(scanActs)` reflection (worker_set.go:246) — confirmed no missing-activity-registration bug, and `module_domains_test.go`'s real-engine test still exercises that registration path. ### Budget cap correctness — PASS - By-ids trigger (internal/app/build_wf_triggers.go:298-311) hard-rejects when `len(bookIDs) > maxBooks`, wrapping `middleware.ErrValidation` (→ 400). - By-filter/whole-library enforces the running total across `ContinueAsNew` epochs via `ScanFileProcessed` carried forward (bulk_by_filter_workflow.go:266-286); boundary-exact and mid-epoch-truncation cases are both covered by non-vacuous multi-epoch tests (`bulk_by_filter_workflow_test.go:1669`, `:1749`). - Default 500 (`DefaultScanFileMaxBooksPerRun`), 0 = default via `EffectiveMaxBooksPerRun`; settings round-trip and validation (`[0, 100000]`) covered in `scan_file_settings_test.go`. ### Metrics — PASS `registerScanFileCounter` (internal/books/scan_file_metrics.go) returns the existing collector on `AlreadyRegisteredError`, safe for repeated `app.New` calls; non-vacuous counter-value tests via `reg.Gather()`. ### Findings [MAJOR] internal/app/build_wf_triggers.go:298-311 — by-ids budget-reject guardrail has zero test coverage `internal/app` is (by project convention) untested at the unit level and relies on e2e tests to prove correctness ("pure wiring; no unit tests by design — correctness verified by e2e"). This PR adds real conditional business logic here — the per-run budget hard-reject for `StartBulkScanFileByIDs` — but the diff adds no e2e spec (browser or API) exercising it, and none of the existing scan-file e2e journeys (`journey_bulk_scan_file_test.go`, `journey_scan_file_reviews_test.go`, `journey_library_scan_file_test.go`) were touched. This is the primary cost-guardrail the bead exists to add, and it is currently unverified by any automated test — a typo or logic inversion (e.g. `<` vs `>`) here would ship silently. Add at least one e2e assertion that POSTing a by-ids selection larger than the configured `max_books_per_run` returns 400/validation-error and does not start the workflow. ### Minor observations (non-blocking) [MINOR] static/js/controllers/library_kebab_menu_controller.js:398-409 — `_scanFileEstimate` uses a hardcoded `20` sec/book constant with no relation to any measured or server-configured value. It's documented as "rough"/"dry-run preview," which is honest, but if the LLM per-book latency changes materially the estimate will silently drift out of date with no test or config tying it to reality. Consider a code comment pointer or follow-up bead to revisit if provider latency changes significantly. [MINOR] internal/settings/scan_file_settings.go:210 — `ErrInvalidMaxBooksPerRun` is built with `fmt.Errorf` (no `%w`) as a sentinel `var`, unlike its sibling `ErrInvalidThreshold` which uses `errors.New`. Cosmetic only (both work identically as comparison sentinels since there's no wrapped verb), but slightly inconsistent style — prefer `errors.New` for a pure sentinel. REVIEW VERDICT: 0 blocker, 1 major, 2 minor
zombor commented 2026-09-07 02:30:02 +00:00
Author
Owner
Copy link

Code Review — PR #1475 (bd-bookshelf-cs2zl.4)

Reviewed the diff only (no re-run of make test/coverage/e2e).

Workflow replay-safety (highest priority) — PASS

  1. BulkByFilterInput.ScanFileMaxBooks/ScanFileProcessed (internal/wfengine/bulk_by_filter_workflow.go:110-121): scanFileBudgetTruncate (line ~262) is gated on input.Op != BulkFilterOpScanFile || input.ScanFileMaxBooks <= 0 — for every non-scan-file op, and for pre-deploy in-flight scan-file instances whose recorded input predates the field (unmarshals to 0), it returns bookIDs unchanged and exhausted=false, so the emitted command sequence (ExecuteActivity/fan-out/ContinueAsNew) is byte-identical to before. Confirmed correctly gated — not a hazard.
  2. ScanFileBookInput.EmitSkipMetric (internal/wfengine/bulk_scan_file_workflow.go:80-88): the new RecordSkip ExecuteActivity (line ~191) is gated on input.EmitSkipMetric, which defaults to false on unmarshal for any in-flight sub-workflow instance recorded before this field existed — replay-safe. New parent-workflow epochs setting it true unconditionally for new sub-workflow instances does not affect the parent's own replay determinism (only sub-workflow input payload content changes, not the parent's command type/order). RecordSkip is a method on the already-registered BulkScanFileActivities struct, registered automatically via ws.registerActivity(scanActs) reflection (worker_set.go:246) — confirmed no missing-activity-registration bug, and module_domains_test.go's real-engine test still exercises that registration path.

Budget cap correctness — PASS

  • By-ids trigger (internal/app/build_wf_triggers.go:298-311) hard-rejects when len(bookIDs) > maxBooks, wrapping middleware.ErrValidation (→ 400).
  • By-filter/whole-library enforces the running total across ContinueAsNew epochs via ScanFileProcessed carried forward (bulk_by_filter_workflow.go:266-286); boundary-exact and mid-epoch-truncation cases are both covered by non-vacuous multi-epoch tests (bulk_by_filter_workflow_test.go:1669, :1749).
  • Default 500 (DefaultScanFileMaxBooksPerRun), 0 = default via EffectiveMaxBooksPerRun; settings round-trip and validation ([0, 100000]) covered in scan_file_settings_test.go.

Metrics — PASS

registerScanFileCounter (internal/books/scan_file_metrics.go) returns the existing collector on AlreadyRegisteredError, safe for repeated app.New calls; non-vacuous counter-value tests via reg.Gather().

Findings

[MAJOR] internal/app/build_wf_triggers.go:298-311 — by-ids budget-reject guardrail has zero test coverage
internal/app is (by project convention) untested at the unit level and relies on e2e tests to prove correctness ("pure wiring; no unit tests by design — correctness verified by e2e"). This PR adds real conditional business logic here — the per-run budget hard-reject for StartBulkScanFileByIDs — but the diff adds no e2e spec (browser or API) exercising it, and none of the existing scan-file e2e journeys (journey_bulk_scan_file_test.go, journey_scan_file_reviews_test.go, journey_library_scan_file_test.go) were touched. This is the primary cost-guardrail the bead exists to add, and it is currently unverified by any automated test — a typo or logic inversion (e.g. < vs >) here would ship silently. Add at least one e2e assertion that POSTing a by-ids selection larger than the configured max_books_per_run returns 400/validation-error and does not start the workflow.

Minor observations (non-blocking)

[MINOR] static/js/controllers/library_kebab_menu_controller.js:398-409 — _scanFileEstimate uses a hardcoded 20 sec/book constant with no relation to any measured or server-configured value. It's documented as "rough"/"dry-run preview," which is honest, but if the LLM per-book latency changes materially the estimate will silently drift out of date with no test or config tying it to reality. Consider a code comment pointer or follow-up bead to revisit if provider latency changes significantly.

[MINOR] internal/settings/scan_file_settings.go:210 — ErrInvalidMaxBooksPerRun is built with fmt.Errorf (no %w) as a sentinel var, unlike its sibling ErrInvalidThreshold which uses errors.New. Cosmetic only (both work identically as comparison sentinels since there's no wrapped verb), but slightly inconsistent style — prefer errors.New for a pure sentinel.

REVIEW VERDICT: 0 blocker, 1 major, 2 minor

## Code Review — PR #1475 (bd-bookshelf-cs2zl.4) Reviewed the diff only (no re-run of make test/coverage/e2e). ### Workflow replay-safety (highest priority) — PASS 1. `BulkByFilterInput.ScanFileMaxBooks`/`ScanFileProcessed` (internal/wfengine/bulk_by_filter_workflow.go:110-121): `scanFileBudgetTruncate` (line ~262) is gated on `input.Op != BulkFilterOpScanFile || input.ScanFileMaxBooks <= 0` — for every non-scan-file op, and for pre-deploy in-flight scan-file instances whose recorded input predates the field (unmarshals to 0), it returns `bookIDs` unchanged and `exhausted=false`, so the emitted command sequence (`ExecuteActivity`/fan-out/`ContinueAsNew`) is byte-identical to before. Confirmed correctly gated — not a hazard. 2. `ScanFileBookInput.EmitSkipMetric` (internal/wfengine/bulk_scan_file_workflow.go:80-88): the new `RecordSkip` `ExecuteActivity` (line ~191) is gated on `input.EmitSkipMetric`, which defaults to `false` on unmarshal for any in-flight sub-workflow instance recorded before this field existed — replay-safe. New parent-workflow epochs setting it `true` unconditionally for new *sub-workflow* instances does not affect the parent's own replay determinism (only sub-workflow input payload content changes, not the parent's command type/order). `RecordSkip` is a method on the already-registered `BulkScanFileActivities` struct, registered automatically via `ws.registerActivity(scanActs)` reflection (worker_set.go:246) — confirmed no missing-activity-registration bug, and `module_domains_test.go`'s real-engine test still exercises that registration path. ### Budget cap correctness — PASS - By-ids trigger (internal/app/build_wf_triggers.go:298-311) hard-rejects when `len(bookIDs) > maxBooks`, wrapping `middleware.ErrValidation` (→ 400). - By-filter/whole-library enforces the running total across `ContinueAsNew` epochs via `ScanFileProcessed` carried forward (bulk_by_filter_workflow.go:266-286); boundary-exact and mid-epoch-truncation cases are both covered by non-vacuous multi-epoch tests (`bulk_by_filter_workflow_test.go:1669`, `:1749`). - Default 500 (`DefaultScanFileMaxBooksPerRun`), 0 = default via `EffectiveMaxBooksPerRun`; settings round-trip and validation (`[0, 100000]`) covered in `scan_file_settings_test.go`. ### Metrics — PASS `registerScanFileCounter` (internal/books/scan_file_metrics.go) returns the existing collector on `AlreadyRegisteredError`, safe for repeated `app.New` calls; non-vacuous counter-value tests via `reg.Gather()`. ### Findings [MAJOR] internal/app/build_wf_triggers.go:298-311 — by-ids budget-reject guardrail has zero test coverage `internal/app` is (by project convention) untested at the unit level and relies on e2e tests to prove correctness ("pure wiring; no unit tests by design — correctness verified by e2e"). This PR adds real conditional business logic here — the per-run budget hard-reject for `StartBulkScanFileByIDs` — but the diff adds no e2e spec (browser or API) exercising it, and none of the existing scan-file e2e journeys (`journey_bulk_scan_file_test.go`, `journey_scan_file_reviews_test.go`, `journey_library_scan_file_test.go`) were touched. This is the primary cost-guardrail the bead exists to add, and it is currently unverified by any automated test — a typo or logic inversion (e.g. `<` vs `>`) here would ship silently. Add at least one e2e assertion that POSTing a by-ids selection larger than the configured `max_books_per_run` returns 400/validation-error and does not start the workflow. ### Minor observations (non-blocking) [MINOR] static/js/controllers/library_kebab_menu_controller.js:398-409 — `_scanFileEstimate` uses a hardcoded `20` sec/book constant with no relation to any measured or server-configured value. It's documented as "rough"/"dry-run preview," which is honest, but if the LLM per-book latency changes materially the estimate will silently drift out of date with no test or config tying it to reality. Consider a code comment pointer or follow-up bead to revisit if provider latency changes significantly. [MINOR] internal/settings/scan_file_settings.go:210 — `ErrInvalidMaxBooksPerRun` is built with `fmt.Errorf` (no `%w`) as a sentinel `var`, unlike its sibling `ErrInvalidThreshold` which uses `errors.New`. Cosmetic only (both work identically as comparison sentinels since there's no wrapped verb), but slightly inconsistent style — prefer `errors.New` for a pure sentinel. REVIEW VERDICT: 0 blocker, 1 major, 2 minor
fix(bulk-scan): test the by-ids budget reject, fix minor review nits (bookshelf-cs2zl.4)
All checks were successful
/ JS Unit Tests (pull_request) Successful in 1m2s
Details
/ E2E API (pull_request) Successful in 1m13s
Details
/ Test Race (pull_request) Successful in 1m46s
Details
/ Lint (pull_request) Successful in 2m10s
Details
/ Coverage (pull_request) Successful in 2m35s
Details
/ Integration (pull_request) Successful in 2m38s
Details
/ E2E Browser (pull_request) Successful in 5m13s
Details
d3925dea8b
 
Extract the over-budget reject into ScanFileSettings.CheckMaxBooksBudget,
a pure function unit-tested black-box in internal/settings (internal/app
stays untested wiring per convention). build_wf_triggers.go now delegates
to it instead of constructing the error inline.

Also: ErrInvalidMaxBooksPerRun now uses errors.New like its sibling
sentinels (was fmt.Errorf with no format args), and the scan-file dry-run
per-book time estimate constant is hoisted to a named, commented
module-level constant instead of being redeclared per call.
All checks were successful
/ JS Unit Tests (pull_request) Successful in 1m2s
Details
/ E2E API (pull_request) Successful in 1m13s
Required
Details
/ Test Race (pull_request) Successful in 1m46s
Required
Details
/ Lint (pull_request) Successful in 2m10s
Required
Details
/ Coverage (pull_request) Successful in 2m35s
Required
Details
/ Integration (pull_request) Successful in 2m38s
Required
Details
/ E2E Browser (pull_request) Successful in 5m13s
Required
Details
This pull request is blocked because it's outdated.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin bd-bookshelf-cs2zl.4:bd-bookshelf-cs2zl.4
git switch bd-bookshelf-cs2zl.4
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
zombor/pergamum!1475
No description provided.