Conversation
- sort each page's file attachments instead of leaking set iteration order - restore the configured page order before the loop that claims files - keep Item: entries aligned with config.titles, no full-array sort - add two offline regression tests covering both ordering sources Closes #172
Contributor
Release previewMerging this PR would release v2.3.6 (current: Changelog preview (truncated)## v2.3.6 (2026-09-21)
### Bug Fixes
- **package**: Write packages.json pages in a deterministic order
([`467d11c`](https://github.com/OpenSemanticLab/osw-python/commit/467d11cc26a0a94f3a31936124740f33c31692c0))
Preview via python-semantic-release and conventional commits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #172
Cause
The issue attributes the churn to the parallel page fetch appending attachments in completion order. That is not the mechanism:
file_dumpsis only read by key atsrc/osw/wtsite.py:1005and:1007, never iterated, so its insertion order never reaches the output. There are three real sources:src/osw/wtsite.py:2228-find_file_page_refs_in_slotsreturnslist(set(file_page_refs)). CPython randomizes string hashes per process, so this order varies between runs.src/osw/wtsite.py:967-972-page_files[page.title] = list(set(referenced_file_pages) - set(added_file_titles)). A second set round-trip. This is the list the final assembly loop walks, so this alone explains the measured churn.src/osw/wtsite.py:919-942-pagesis filled bypages.appendinside a thread pool (:417, viaparallelizeat:446). The loop at:947accumulatesadded_file_titleswhile walkingpages, so when two configured pages reference the same file, which page claims it varies per run and the file moves in the output.Source 3 also appears without parallel fetching:
src/osw/wtsite.py:926-928doeslist(set(titles_to_fetch) - set(param.offline_pages.keys())), so the fetched pages arrive in set order wheneveroffline_pagesis supplied.Changes
page_files[page.title]now usessorted()instead oflist(). Makes each page's attachments alphabetical byFile:title and neutralizes sources 1 and 2. This is the fix proposed in the issue.pagesis sorted by rank inadded_titlesbefore the loop that claims files. Fixes source 3.added_titlescomes fromlist(dict.fromkeys(config.titles))and is already deterministic.tests/test_wtsite_create_page_package.py.The commented-out
bundle.packages[config.name].pages.sort(key=lambda x: x.urlPath)atsrc/osw/wtsite.py:1010is left as is. Enabling it would reorder theItem:entries byurlPathand lose their correspondence withconfig.titles, as the issue notes.find_file_page_refs_in_slotsis unchanged. It is public API, and thesorted()change makes its order irrelevant to this output.Rationale for the shared-file rule
When several pages reference the same file, it is now always attached to the page that comes first in
config.titles. This is not a new rule: the existing- set(added_file_titles)filter atsrc/osw/wtsite.py:971already means the first page to reference a file wins. The change makes that rule deterministic instead of dependent on thread completion order.Verification
PYTHONHASHSEED0, 1, 42, 12345 and 99999.ruff checkandruff format --checkclean on both changed files.Not verified against a live wiki; I had no credentials. Two consecutive builds of
world.opensemantic.meta.docswould confirm the checksum is now stable.Known limit of the tests
The tests substitute
WtPage.dumpwith a stand-in returning onlyname,namespaceandslots, and compare only thenamesequence. That is sufficient for an ordering test, because the change never alters a dump's contents, only which dump object goes where. The tests would be blind only if two entries shared aname.