Skip to content

Fix warm binary request cache hits - #44

Merged
pirate merged 4 commits into
mainfrom
fix/warm-cache-hit-performance
Aug 31, 2026
Merged

Fix warm binary request cache hits#44
pirate merged 4 commits into
mainfrom
fix/warm-cache-hit-performance

Conversation

@pirate

@pirate pirate commented Aug 30, 2026

Copy link
Copy Markdown
Member

Summary

  • make absolute-path requests reuse the same validated projection as their cached basename record, using identical normalized path identity on read and write
  • validate managed and global pnpm packages against the installed package.json version instead of accepting a desired literal override as observed state
  • verify global package ownership through the real pnpm launcher target before trusting package metadata
  • invalidate stale pnpm records when the cached version differs from the installed package

This fixes the root cache misses behind repeated ArchiveBox plugin installation checks. It does not add skip logic or parallelize the ordered installer.

Root causes

  • request projections were written under basename identities but looked up using absolute request names
  • the read and write sides normalized absolute paths differently
  • literal pnpm version overrides could replace observed package metadata, allowing a stale image dependency to appear current
  • global pnpm records skipped installed-version validation and did not verify package ownership
  • none of these cases had real regression coverage

Verification

  • absolute-path projection tests prove second real BinaryService requests, including a normalized .. path, never enter _load
  • a real pnpm test installs zx 7.2.3 with a conflicting literal 999.0.0 override and pins package.json plus the returned Binary to 7.2.3
  • a real pnpm-style global shell launcher test resolves its package target, ignores unrelated nearer metadata, and validates the owning package without spawning pnpm
  • focused cache regressions: 4 passed
  • changed-file prek hooks, ty, and pyright: passed
  • full related pair before the final ownership tightening: 55 passed, 1 skipped; six macOS /var vs /private/var assertions also fail unchanged on pristine main

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread abxpkg/binprovider_pnpm.py Outdated
Comment thread abxpkg/binprovider.py Outdated
Comment thread tests/test_pnpmprovider.py Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread abxpkg/binprovider_pnpm.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread abxpkg/binprovider_pnpm.py
@pirate
pirate merged commit ea5de2c into main Aug 31, 2026
39 checks passed
pirate added a commit to ArchiveBox/ArchiveBox that referenced this pull request Sep 1, 2026
## Summary

- make `archivebox list --html` write every snapshot detail page from
the same `core/snapshot.html` Django template used for live admin site
- create canonical UUID detail directories for pending/unarchived
snapshots too, without embedding live admin routes
- omit stale/nonexistent output targets, suppress bogus `/None` icons,
and correctly URL-encode literal `%` filenames
- create non-TTY crawl progress reporters and relay supervised worker
output on stderr so JSONL stdout stays valid
- replace both public-site Docker examples of `init --install` with
plain `init` and remove their false install output

Closes #1847.

## Issue reproduction

Built 0.9.35rc352 from source, ran a fresh Docker collection with only
`archivebox init`, then archived `https://sweeting.me/` at depth 1. The
completed collection has 75 snapshots, 14.0 GB, and only canonical
`archive/users/system/snapshots/<date>/<domain>/<uuid>/` roots鈥攝ero
legacy timestamp directories.

Before this PR, the exported index linked to 75 missing snapshot detail
pages. After the fix, it contains 76 static HTML pages total. A
filesystem-aware HTML traversal checked 11,286 local href/src targets:
zero missing and zero live `/snapshot`, `/web`, or `/static` routes.

## Verification

- `archivebox/tests/test_cli_list.py`: 23 passed
- `archivebox/tests/test_ui_admin_snapshot.py`: 61 passed
- crawl runner + CLI piping suites: 37 passed
- changed-file prek hooks: passed
- real non-TTY Docker add emitted process STARTED and succeeded rows
while preserving stdout/stderr separation
- unarchived-snapshot regression starts with a missing output directory
and proves export creates a portable detail page with no live admin URL

## Dependency PRs

- ArchiveBox/abx-dl#18 supplies the shared non-TTY lifecycle renderer
- ArchiveBox/abxpkg#44 fixes the warm installer cache identity/version
bugs found during this reproduction

No installer concurrency or skip branches were added. The non-Docker
development/CLI `init --install` examples and its real fuzz test are
intentionally unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant