Skip to content

Reduce backend duplication and strengthen service boundaries - #79

Merged
abhi1693 merged 4 commits into
masterfrom
refactor/backend-service-boundaries
Oct 4, 2026
Merged

abhi1693 merged 4 commits into
masterfrom
refactor/backend-service-boundaries

Conversation

@abhi1693

@abhi1693 abhi1693 commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

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.

  • Compose API lifecycle, admission, logging, errors and health routes through 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.
  • Replace the 22-complexity claimed article-analysis workflow with 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.
  • Move notification expiry, retry and lease recovery into DeliveryPolicy in core. The scheduler no longer imports the notification delivery application; manual retries and delivery use the same inclusive 28-day idempotency safety window.
  • Make notification delivery an optional aggregator dependency. Dedicated article-enrichment, image and source-discovery workers omit it. [all-workers] and the existing CLI/default backend image retain it; a worker missing the adapter gives an actionable error before dequeueing.
  • Add import-boundary checks for every Python workspace member and fresh-process API import checks.

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.

  • Full local unit/report pipeline before the readiness follow-up: 3,001 passed, no failures/skips, all 273 backend source files, 67.71% combined coverage with the 67.60% minimum enforced.
  • 42 real PostgreSQL 18 / Redis 8 integration regressions passed, including catalog writers committing while ranking is paused, changed input/ownership rejection, bounded churn, scheduler reuse, notification recovery and API health.
  • 60 focused scheduler/lifecycle/boundary/delivery-policy unit cases passed; one database integration case was not configured in that focused command and remains a hosted-CI gate. The old scheduler mock now targets core's recovery owner.
  • Python checks and Ruff C901 maximum 10 passed; admin contract regeneration produced no changes. New architecture-planning Markdown and its README link are removed; remaining decisions are in this description.
  • Six new ASGI readiness regressions pass for all three APIs: healthy and unhealthy responses both release the overridden database session before response transmission. Lifecycle/readiness suite: 17 passed. Python lint and type checks pass.
  • The first rebased CI run passed its other checks but Sonar reported an unsupported scope argument even though locked FastAPI 0.142.2 supports it. Readiness now reuses each API’s explicit named dependency, preserving scope="function" and override identity. No gate, rule, exclusion or coverage threshold is weakened.
  • Normal commit hooks passed. Current-head CI passed all 42 jobs, with 13 expected skips, including 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.
  • Hosted backend unit tests: 3,007 passed, no skips, 273 source files, 67.72% combined coverage against the unchanged 67.60% minimum. All 1,137 PostgreSQL/Redis integration tests passed across three shards; web 818 and admin 648 unit tests passed.
  • Sonar MCP confirms the current PR gate is OK, 98.2% new-code coverage, 0.0% new duplication and zero open issues. The unsupported-argument report is gone without changing any rule or gate. Auto-merge remains disabled.
  • GitHub’s separate managed AI findings scanner remains unavailable because its monthly quota is exhausted (HTTP 402); standard CodeQL passed. No paid-tier or account changes were made.
  • An inherited heartbeat fixture replaces threading.Event globally 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

  • The description explains the change and validation results, including gaps.
  • Relevant documentation and regression tests are updated.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

PR reader API performance

Result: INCONCLUSIVE

8 inconclusive. Zero request failures across all six runs.

Base 9ba10cedf7980801c09d875b67a19f881dbdbbe0 → PR 22c5f0ec980f9ebbed189b9bc2d421c3c815b36e

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 comparison

All latency values are in milliseconds; lower is better. Values are medians of three runs.

Route Base p95 PR p95 Change Result
All requests 99 80 -19 (-19.2%) Inconclusive
Latest feed 120 100 -20 (-16.7%) Inconclusive
Feed pagination 92 71 -21 (-22.8%) Inconclusive
Article detail 92 55 -37 (-40.2%) Inconclusive
Source feed 90 77 -13 (-14.4%) Inconclusive
Topic feed 90 83 -7 (-7.8%) Inconclusive
Feed filters 71 70 -1 (-1.4%) Inconclusive
Sources 44 33 -11 (-25.0%) Inconclusive
Topics 43 45 +2 (+4.7%) Inconclusive
Additional latency and throughput metrics

Each cell shows base → PR. Latencies are in milliseconds.

Endpoint p50 p99 Requests/s
Aggregated 29 → 25 140 → 120 29.41 → 30.50
/v1/feed [latest] 46 → 39 180 → 130 8.84 → 9.09
/v1/feed [page] 26 → 23 120 → 110 3.65 → 3.63
/v1/articles/[id] 18 → 15 140 → 86 6.15 → 6.12
/v1/feed [source] 31 → 25 110 → 110 1.91 → 1.90
/v1/feed [topic] 27 → 25 120 → 130 1.82 → 1.67
/v1/feed/options 21 → 19 120 → 98 2.44 → 2.76
/v1/sources 12 → 10 66 → 55 2.44 → 2.76
/v1/topics 12 → 11 80 → 63 2.44 → 2.76
Individual runs
Run Requests Failures Exit code
Base 1 1715 0 0
Base 2 1649 0 0
Base 3 1649 0 0
PR 1 1728 0 0
PR 2 1714 0 0
PR 3 1707 0 0
How to interpret this report

Regressed / 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

@abhi1693
abhi1693 force-pushed the refactor/backend-service-boundaries branch from b682bd0 to 26bff19 Compare October 4, 2026 15:20
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Backend unit coverage

PASS · minimum combined coverage 67.60%.

Tests: 3007 passed, 0 failed, 0 skipped. Source files: 273.

Workspace Lines Branches Combined
All backend code 71.57% 53.67% 67.72%
apps/admin-api 64.78% 35.76% 59.95%
apps/aggregator 73.60% 57.18% 69.86%
apps/api 68.94% 55.88% 66.48%
apps/article-enrichment-worker 80.00% 50.00% 71.43%
apps/cli 68.76% 42.93% 64.81%
apps/images-worker 80.00% 50.00% 71.43%
apps/mcp 79.90% 73.88% 78.77%
apps/notifications 83.33% 76.67% 81.75%
apps/search-indexer 85.48% 85.71% 85.53%
apps/source-discovery-worker 80.00% 50.00% 71.43%
apps/user-api 59.19% 32.90% 54.27%
packages/core 73.66% 56.49% 69.57%
packages/http 94.43% 86.47% 92.62%

Unit coverage includes every backend workspace. PostgreSQL/Redis integration tests run separately.

Commit: 22c5f0e · HTML, JSON, XML reports and job logs · attempt 1

@gitar-bot

gitar-bot Bot commented Oct 4, 2026

Copy link
Copy Markdown
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 HTTPService with explicit named database dependencies, replacing the 22-complexity article-analysis workflow with ArticleAnalysisService (max method complexity 6), and moving notification policy into core while making delivery an optional dependency. Validation includes 3,001 passed unit/report tests at 67.71% coverage, 42 PostgreSQL/Redis integration regressions, and 60 focused lifecycle/boundary cases. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

sonarqubecloud Bot commented Oct 4, 2026

Copy link
Copy Markdown

@abhi1693
abhi1693 merged commit 626947d into master Oct 4, 2026
58 checks passed
@abhi1693
abhi1693 deleted the refactor/backend-service-boundaries branch October 4, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant