Skip to content

fix(themes): stop archived theme pages from blocking their entity set - #710

Merged
plind-junior merged 4 commits into
vouchdev:testfrom
dripsmvcp:fix/themes-archived-pages
Jul 31, 2026
Merged

fix(themes): stop archived theme pages from blocking their entity set#710
plind-junior merged 4 commits into
vouchdev:testfrom
dripsmvcp:fix/themes-archived-pages

Conversation

@dripsmvcp

Copy link
Copy Markdown
Contributor

closes #704

what

themes._existing_theme_entity_sets walked store.list_pages() with no
lifecycle filter, so an archived theme page kept contributing its entity
frozenset and propose_theme treated that cluster as already taken.

Archive is how an operator retires a wrong theme — and this made the
retirement permanent for that entity cluster. The only way to re-synthesize it
was hand-editing storage. Same one-way trap compile's TAKEN TOPICS had
(#700).

Claims were already excluded here through _EXCLUDED_STATUSES, which includes
ARCHIVED. Theme pages were not.

the fix

Skip PageStatus.ARCHIVED when building the existing entity sets. The
type == "theme" filter the issue also suggests was already in place, so the
diff is the one lifecycle check.

Pending proposals deliberately still count: awaiting review is not retirement,
and two proposals for one cluster is exactly the duplicate this guard exists to
prevent.

tests

Four in tests/test_themes.py:

  • an archived theme page no longer contributes its entity set (the issue's
    repro)
  • a live one still does — the filter is archive-only
  • a pending proposal still blocks, so the dedupe guard is not weakened
  • end to end: propose a theme, approve it, confirm detect_themes dedupes the
    cluster away, archive the page, and confirm the cluster resurfaces

Full gate green locally: pytest tests/ --ignore=tests/embeddings, mypy src,
ruff check src tests, plus diff-cover --fail-under 100 on the changed lines.

Sibling of #700 (compile) and #701 (provenance graph) — same archive leak,
three different surfaces, one PR each and no file overlap between them.

`_existing_theme_entity_sets` walked every page with no lifecycle filter,
so an archived theme page kept contributing its entity frozenset and
`propose_theme` treated that cluster as already taken after archive.

archive is how an operator retires a wrong theme, and this made the
retirement permanent for that entity cluster — the only way to
re-synthesize it was hand-editing storage. same one-way trap compile's
TAKEN TOPICS had (vouchdev#700).

claims were already excluded through `_EXCLUDED_STATUSES`, which includes
ARCHIVED; theme pages were not. skip archived pages when building the
existing sets.

pending proposals still count. awaiting review is not retirement, and two
proposals for one cluster is the duplicate this guard exists to prevent.
@github-actions github-actions Bot added tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 31, 2026
@plind-junior

Copy link
Copy Markdown
Member

read this alongside #709, #708, #698 and #697 — five PRs, one bug class, and the cluster is worth a word on its own.

the fix here is right and the test set is the best of the five: the end-to-end case (propose → approve → confirm dedupe → archive → confirm the cluster resurfaces) is the one that actually proves the user-visible behaviour rather than just the predicate. holding pending proposals in the guard is also the correct call, and it is good that a test pins it.

one cross-cutting note. context._page_is_live already exists and its docstring is explicit about why: "pages are only ever tested, so the check lives here once — keeping it in three places is what let kb.context keep serving archived pages after #581 fixed kb.search." this cluster now adds five more copies of that predicate — an inline is PageStatus.ARCHIVED here, another in provenance/graph.py, a _live_pages helper in compile.py, a near-verbatim copy of _page_is_live in graph._neighbor_ok, and a list comprehension in cli.render_wiki_cmd. the sixth surface will be a sixth copy.

_live_pages(store) from #708 is the shape that fits three of the five — the ones iterating store.list_pages() and already holding the Page. suggest promoting it next to _page_is_live as page_is_live(page: Page) -> bool, with the store-and-id form delegating to it, and landing this cluster on it so the next surface is a one-line import rather than a sixth reimplementation.

nothing blocking in this diff.

@github-actions
github-actions Bot disabled auto-merge July 31, 2026 06:55
@plind-junior
plind-junior enabled auto-merge July 31, 2026 12:40
@github-actions
github-actions Bot disabled auto-merge July 31, 2026 16:08
@plind-junior
plind-junior merged commit afcc40f into vouchdev:test Jul 31, 2026
11 checks passed
@github-actions github-actions Bot added the ci: passing ci is green label Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

diff coverage: n/a — this PR changes no python under src/vouch/, so there is nothing for the gate to measure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: passing ci is green size: S 50-199 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(themes): archived theme pages still block entity-set dedupe

2 participants