Repository navigation
Reduce backend duplication and strengthen service boundaries - #79
Conversation
PR reader API performanceResult: INCONCLUSIVE 8 inconclusive. Zero request failures across all six runs. Base Three independent runs per revision, each on a fresh GitHub-hosted ARM64 runner. 16 users, 60 seconds/run, 1,000 synthetic articles, cache off, five PgBouncer slots. Latency comparisonAll latency values are in milliseconds; lower is better. Values are medians of three runs.
Additional latency and throughput metricsEach cell shows base → PR. Latencies are in milliseconds.
Individual runs
How to interpret this reportRegressed / Improved: a p95 regression needs >20% and >20 ms increase in the medians and at least two head samples versus the base median. Improvements use the inverse threshold. Within threshold means the change did not meet both thresholds. Inconclusive: a within-revision p95 range >35% of its median and >20 ms is too noisy for a confident latency verdict. Inconclusive latency comparisons are non-blocking: the check passes without claiming an improvement. Confirmed regressions and test/report errors fail the PR gate. Throughput is descriptive: paced users do not measure maximum capacity. This uncached public API test does not establish production health, browser performance or worker capacity. Separate runners can differ in hardware or host load; repetitions and noise checks reduce but cannot eliminate that uncertainty. Full HTML/CSV reports and logs · attempt 1 |
b682bd0 to
26bff19
Compare
Backend unit coveragePASS · minimum combined coverage 67.60%. Tests: 3007 passed, 0 failed, 0 skipped. Source files: 273.
Unit coverage includes every backend workspace. PostgreSQL/Redis integration tests run separately. Commit: 22c5f0e · HTML, JSON, XML reports and job logs · attempt 1 |
Code Review ✅ Approved🟡 Medium risk · Reworks article-analysis transactions, API lifecycle cleanup, and notification recovery policy. Refactor reduces backend duplication by composing API lifecycle through OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|



What changes and why?
Backend services already run separately, but repeated lifecycle wiring and scheduler imports still couple them. For example, an admin Codex startup failure previously bypassed shutdown, and a failed snapshot/client close prevented later resources from being released.
HTTPService. Each API exposes an explicit named database dependency shared by its routes and readiness probe, with no annotation indexing. Cleanup now attempts every registered resource even when startup or another close fails.ArticleAnalysisService, an injected client protocol and explicit prepare/infer/propose/apply/fail phases. Its most complex method scores 6. Lease ownership, source/catalog checks, validation feedback and attempt accounting remain enforced; database retries do not repeat inference.DeliveryPolicyin core. The scheduler no longer imports the notification delivery application; manual retries and delivery use the same inclusive 28-day idempotency safety window.[all-workers]and the existing CLI/default backend image retain it; a worker missing the adapter gives an actionable error before dequeueing.Validation
Rebased after #72 and against master through #78, following this PR's merge instructions. The analysis service retains computation outside transactions, no checked-out connection during ranking, locked content/catalog revision guards, bounded three-attempt preparation and inference that is not repeated on database retries. Both source and catalog race regressions remain in the branch.
scopeargument even though locked FastAPI 0.142.2 supports it. Readiness now reuses each API’s explicit named dependency, preservingscope="function"and override identity. No gate, rule, exclusion or coverage threshold is weakened.CI required, all three CodeQL analyses, SonarQube, dependency/secret/workflow checks, populated upgrades, web/Chrome/Edge parity, admin/login browser journeys, performance comparison and ARM64 image build/scan/smoke checks.threading.Eventglobally and can interrupt asynchronous test logging. Its test-only warning will be fixed in a separate PR after this one and before Remove obsolete backend and frontend code #80; no production threading changes are needed.Rollout notes
No schema, public API, version or deployment topology change. Existing durable RQ handler paths and notification helper imports remain compatible. Lightweight installations consuming notifications must explicitly install
devfeed-aggregator[notifications]or[all-workers]; the default backend image already receives[all-workers]through the CLI.This is an incremental service-boundary refactor. PostgreSQL and domain models remain shared. Further database separation needs owner-specific writes, versioned events, idempotent consumers, backfill/reconciliation and separate runtime credentials. Follow-up architecture decision: select the first data owner to extract; account preferences/engagement are a reasonable starting point. Large Codex transport, discovery and topic-research workflows still deserve separate refactors.
Implemented in the original checkout on a separate branch from
master. Other audit/security/performance/UX work and backend coverage CI changes remain in their separate PRs. Production has not been deployed or modified. The required #72-first order is satisfied. This branch is rebased on master after #78 and preserves the lock fix; its PostgreSQL lock regressions pass. Merge this PR next, followed by the separate heartbeat-fixture fix before continuing #80.Ready for review