Skip to content

Encapsulate WG21Index.papers; enforce access via existing snapshot accessors #88

Description

@henry0816191

Problem

WG21Index.papers is a public, directly-mutable dict[str, Paper] (src/paperscout/sources.py line 52; the adjacent comment at line 51 says "Replaced wholesale on every refresh(); never mutate in place"). The class docstring (lines 36–44) states the class is single-threaded-async-only and warns that refresh() "would race with cross-thread reads on papers / _max_rev" — i.e. the invariant is enforced only by convention, not by the type system. refresh() publishes a new map via _commit_index() (self.papers = papers, line 183) and returns IndexRefreshResult(self.papers, …) (lines 65/74/91).

Read-only accessors already exist but are not used consistently:

  • get_known_paper_ids()frozenset (lines 227–229)
  • get_papers_snapshot()MappingProxyType (lines 231–233)

Yet the live attribute is still read directly. The load-bearing cross-thread reader is paper_count_fn = lambda: len(index.papers) in src/paperscout/__main__.py (line 250), which is handed to the health server (registered at lines 285–286) and Slack handlers/startup status (lines 279, 299) — all of which run off the event loop, so they read index.papers while the loop may reassign it. monitor.Scheduler.seed() also reads len(wg21.papers) directly (src/paperscout/monitor.py lines 367, 372), though that runs on the event loop.

Correction vs. earlier draft: the draft cited Scheduler._previous_papers = dict(self.index.papers) and diff_snapshots(previous, self.index.papers) in monitor.py as the TOCTOU site. Neither exists in the current code — there is no _previous_papers attribute, and the Scheduler diffs via the DataSource protocol: _poll_sources reads current = refresh_result.papers and stores snapshots in self._snapshots, then calls source.diff(previous, current) (monitor.py lines 246, 263–264). So the live defect is the public mutable attribute + cross-thread read via the health/Slack paper_count_fn, not a diff_snapshots re-read.

Acceptance Criteria

  • WG21Index.papers is no longer exposed as a directly-mutable public dict — make the backing store private (e.g. self._papers) and expose reads only through get_papers_snapshot() / get_known_paper_ids() (or a read-only property).
  • In-repo readers are migrated off the live dict: __main__.py paper_count_fn (line 250) and monitor.Scheduler.seed() counts (lines 367/372) use a snapshot/count accessor instead of index.papers.
  • Publication of a new index stays atomic from a reader's perspective (single reference swap in _commit_index, consumed via a snapshot), documented at the swap site.
  • Tests that assign index.papers = index._parse_and_index(...) / index.papers = {...} (tests/test_sources.py, tests/test_monitor.py, tests/test_callback_protocols.py) are updated to the new seam.
  • Tests pass in CI (if applicable)
  • PR approved by at least 1 reviewer

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions