Skip to content

Benchmark scope selection cost - #2145

Open
Aman-goel-04 wants to merge 3 commits into
repowise-dev:mainfrom
Aman-goel-04:benchmark-scope-selection-cost
Open

Benchmark scope selection cost#2145
Aman-goel-04 wants to merge 3 commits into
repowise-dev:mainfrom
Aman-goel-04:benchmark-scope-selection-cost

Conversation

@Aman-goel-04

Copy link
Copy Markdown
Contributor

Summary

  • repowise update marked file pages stale on every commit but never marked module, SCC, or repo-overview pages, so model-written summaries silently drifted from HEAD with no signal anywhere (doctor, get_stale_pages, and generate --stale all reported clean).
  • Reuses the existing scoped-generation cascade (build_dependencies + expand_cascade) to derive those dependents from decay_paths during update, seeding with mode="none" so dependents are marked stale without being regenerated, keeping AUTO_SYNC.md's no-LLM-spend-on-sync promise intact.
  • Adds a benchmark script (scripts/benchmark_scope_selection.py) used to validate this is cheap enough to run on every hook-driven update (~15ms/1.2k files, ~138ms/11k files for the full selection); the numbers are also recorded as a comment at the call site so a future reader doesn't mistake it for an accidental full re-selection.

Related Issues

Fixes #2099

Test Plan

  • Tests pass (pytest)
  • Lint passes (ruff check .)
  • Web build passes (npm run build) (if frontend changes)

Added test_update_decay_paths_use_cascade_dependents, which pins the cascade contract directly: seeds are the file_page: ids from decay_paths, mode is "none" (not "dependents", so containers are never regenerated), and the resulting stale set includes the module/SCC/repo-overview pages.

Checklist

  • My code follows the project's code style
  • I have added tests for new functionality
  • All existing tests still pass
  • I have updated documentation if needed

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @Aman-goel-04. Two things here are exactly what I hoped for: the benchmark script is committed rather than quoted in a comment, so the number is reproducible on someone else's repo, and the measurement landed as a comment next to the call it justifies in scope.py. That comment is the part that will still be doing work in a year, when somebody wonders whether select_all=True on a hook path was an accident.

mode="none" is right and your inline reasoning for it is right: "dependents" would put model spend on every post-commit hook, which is the promise AUTO_SYNC.md makes about sync.

One blocker, and it is in the swap rather than in the cascade.

expand_cascade with mode="none" sets generate = seed_ids and then subtracts it (cascade.py:136-140):

    else:  # "none"
        generate = set(seed_ids)
        stale = containers | repo_wide

    # A page is never both regenerated and marked stale; regeneration wins.
    stale -= generate

So cascade.stale_ids never contains the seeds. The call this replaces marked exactly those seeds: mark_stale_pages(session, repo_id, decay_paths) maps each path to file_page:<path> and downgrades it (persist.py:192-219). After this change the decayed file pages themselves stop being marked, and only their containers and the repo-wide pages are, which turns the bug you set out to fix inside out: update gains concept-page staleness and loses file-page staleness. Nothing in the run marks them afterwards, I checked the rest of _persist_full_update_async.

await mark_page_ids_stale(session, repo_id, cascade.stale_ids | seed_ids) is the whole fix, since compute_page_id("file_page", path) produces the same file_page:<path> string that mark_stale_pages built. A test that asserts a decayed file page is still stale after an update would pin it, and that is the regression worth having a test for regardless of how it is fixed.

One reuse note, not a blocker but do take it: _load_page_rows already exists, in pipeline/scoped_generation.py:86, with the same load_only column list and the same tombstone filter. The new copy in update_cmd/persistence.py is byte-for-byte the same query. Import that one. Its docstring also explains why the column list matters (keeping the content Text column out of a whole-wiki scan), which is the kind of thing that gets lost when a query is copied.

Non-blocking, on the benchmark itself: it measures select_pages plus build_dependencies, but the new code path also calls load_kg_context, which stats and opens .repowise/knowledge-graph.json on every update. Probably nothing, but it is on the hook path now and it is not in the numbers you quoted. Worth one line in the script or one sentence saying you checked it.

Push the seed fix and I will merge.

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.

[Bug] update never marks concept pages stale, so model-written prose drifts with no signal

2 participants