Skip to content

fix(preprocess): unify PR Newswire identities#124

Merged
keli-wen merged 3 commits into
LLMQuant:masterfrom
Sanjays2402:fix/pr-newswire-public-identity
Jul 23, 2026
Merged

fix(preprocess): unify PR Newswire identities#124
keli-wen merged 3 commits into
LLMQuant:masterfrom
Sanjays2402:fix/pr-newswire-public-identity

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

Summary

PR Newswire observations now use the public build_news_identity helper, so callers can reproduce the identity stamped on collected documents. The focused regressions cover both discovery observations and collected documents.

Related Issue

Closes #110

Verification

  • Regression tests failed against the unchanged source and passed with this fix.
  • .venv/bin/python -m pytest -q --no-cov tests/preprocess/test_pr_newswire.py — 10 passed.
  • Ruff format/check and lint-imports passed.
  • basedpyright reports the same 12 unresolved optional-dependency imports with and without this diff in the minimal local environment; bash scripts/verify.sh therefore does not complete locally.
  • The live PR Newswire smoke test was not run in the network-denied local test environment.

Checklist

  • The title uses English Conventional Commit format: type(scope): summary.
  • The related issue or design discussion is linked when applicable.
  • bash scripts/verify.sh passes.
  • Every applicable live-network component smoke test passes, or this PR states why none applies.
  • Public behavior has focused tests, an example, and documentation where applicable.
  • The PR is complete, small, and contains no unrelated changes.

Use the public news identity builder when collecting PR Newswire documents so callers can reproduce stamped identities. Add offline regressions for discovery and collection.
@wanghaoxue0
wanghaoxue0 requested a review from keli-wen July 18, 2026 20:03
@keli-wen keli-wen added type: bug Existing behavior violates its contract or documented expectation area: preprocess Deterministic acquisition, parsing, cleaning, and source handling impact: breaking Can break a public API, contract, or compatibility boundary labels Jul 23, 2026
keli-wen added a commit that referenced this pull request Jul 23, 2026
…outs (#137)

## Summary

The dependency-install step used `uv pip install -e .`, which ignores
the tracked `uv.lock` and re-resolves the whole dependency graph on
every run. On the warm `ci.yml` cache this is fast (~80s), but the
path-filtered/scheduled `e2e.yml` jobs run rarely, so their uv cache is
usually evicted (GitHub drops caches after 7 days of no access). On that
cold cache, resolution alone took ~6m — probing sdist metadata and
backtracking across the 200+ package tree — and the `news` job's
10-minute timeout cancelled the install before it finished, failing e2e
on otherwise-green PRs (for example the run on #124).

This switches every job to `uv sync --frozen` (CI adds `--extra dev`),
which reads the exact pins from `uv.lock` and skips resolution entirely,
and keys the `setup-uv` cache on `uv.lock` instead of `pyproject.toml`
so the cache invalidates on the resolved set rather than the source
manifest. Cold-cache installs now only download the locked wheels,
landing well under the job timeouts; warm CI installs also drop the
redundant re-resolution.

No smoke-test behavior, network dependency, or public API changes — this
is install mechanics only.

## Related Issue

No tracked issue; the failing e2e install surfaced while validating
#124. Root cause and fix are described above.

## Verification

- `bash scripts/verify.sh` — 405 passed, 85.66% coverage, verify loop
passed.
- `uv sync --dry-run` and `uv sync --dry-run --extra dev` both resolve
against the committed `uv.lock` locally, confirming the runtime and dev
sets and the `dev` extra.
- The real cold-cache timing is confirmed by re-running the affected e2e
job after merge.

## Checklist

- [x] The title uses English Conventional Commit format: `type(scope):
summary`.
- [x] The related issue or design discussion is linked when applicable.
- [x] `bash scripts/verify.sh` passes.
- [x] Every applicable live-network component smoke test passes, or this
PR states why none applies — none applies; this changes dependency
install only, not smoke-test or network behavior.
- [x] Public behavior has focused tests, an example, and documentation
where applicable — not applicable; CI-workflow-only change with no
public surface.
- [x] The PR is complete, small, and contains no unrelated changes.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@keli-wen keli-wen 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.

LGTM

@keli-wen
keli-wen merged commit 857b33d into LLMQuant:master Jul 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: preprocess Deterministic acquisition, parsing, cleaning, and source handling impact: breaking Can break a public API, contract, or compatibility boundary type: bug Existing behavior violates its contract or documented expectation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: PR Newswire collector identity diverges from public build_news_identity

2 participants