Repository navigation
Conversation
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This branch has not been deployed
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.
Step 2 of the backend architecture plan.
What moved
app/core/db/, one module per table:publisher.py,article.py,tag.py,story.py,ledger.py,run.py,user.py,like.py,subscriber.py,event.py.UserBase,PublisherBaseandArticleBasemoved with their tables.models.py.app/newsletter/models.pyandpipeline/models.pyheld only tables, so they are gone.record_reject,CONTENT_CAPandCANDIDATE_CAPmoved intoledger.py(waspipeline/rejects.py).record_rejectnow takes aLedgerItemTypedDict, so core does not importpipeline.types. The pipeline'sCandidatestill fits it.kind_from_urlmoved intoarticle.py(waspipeline/url_kind.py).like_counts_subquerymoved intolike.py(was incatalog/articles.py).app/core/db/engine.pyholdsengineandget_session.app/core/db.pyandpipeline/db.pyare gone.deps.SessionDepusesget_session.app/core/db/__init__.pyimports every table module, somigrations/env.pyimportsapp.core.dband needs no per-domain line.app/alembictobackend/migrations. The alembic config is[tool.alembic]inbackend/pyproject.toml.alembic.iniis gone, andenv.pysets up its own logging.migrations.docs/backend.mdpoints at the new paths.tests/pipeline/test_rejects.pymoved totests/core/test_ledger.py.Behaviour changes
pipeline.run,pipeline.report,find_missing_tags) now connects throughapp.core.settings, as ADR 7's follow-up in the plan says. It needsPROJECT_NAME,POSTGRES_SERVERandPOSTGRES_USER. The box already feeds the same.envto every unit.deploy/agentique-pipeline.serviceanddeploy/agentique-report.servicechanged.ExecStartdid not change, so reinstalling the units is optional. Copy them when convenient.prestart.shstill runsalembic upgrade headfrombackend/, which now readspyproject.toml.Bug fixes
fix: percent-encode database credentials in the dsn
PostgresDsn.builddoes not escape the user or password. A password with@,/or#produced a broken URL. The old pipeline engine quoted them, so merging the engines would have broken it. BothSQLALCHEMY_DATABASE_URIandMCP_DATABASE_URInow quote the user and password. Tests are intests/core/test_settings.py.fix: declare the indexes and column types the migrations built
alembic checkfailed on the base branch. The models were missing the threearticleindexes (score, published_at, the hnsw embedding index) andpipeline_run.publishers. They also declared VARCHAR, JSON or naive timestamps where the migrations built TEXT, JSONB and timestamptz (newsletter_subscriber,scored_url.url/created_at,pipeline_run.sources). The models now match the database, so no migration is needed.Tests added
tests/core/test_db.pyruns alembic'scompare_metadataagainst the migrated test db, so CI now fails on schema drift. It also checks that every table lives inapp.core.db, that the engine logs in with the settings credentials, that the request session closes afterwards, and that the routes and the pipeline share one engine.tests/core/test_ledger.pyadds Postgres-backed tests. A missingdetailis stored as SQL NULL. Recording a URL again replaces its row. NUL bytes and an unparsable date still commit.How I verified
alembic upgrade headon an empty db and thenalembic checkboth pass.alembic checkalso passes on the prestart-migrated db.alembic revision --autogenerateproduces an empty revision.pytest: 603 passed./searchfails locally because the HF model download is blocked here. It passes in CI.cd backend && mypy app/core(strict, 0 errors) all pass. Strict mypy overapp scriptsreports 126 errors, which was 127 before. The banned-names hook passes..claude/skills/curate/regression.md, which was already flagged before this PR.scripts/generate-client.shleavesfrontend/src/clientunchanged, and the OpenAPI JSON is identical to the base branch.Generated by Claude Code