Slack credential validation, dead config cleanup, Docker digest pinning#42
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
💤 Files with no reviewable changes (6)
📝 WalkthroughWalkthroughRemoves unused enable_bulk_openstd, adds an early DATABASE_URL startup guard, removes module-level DB pool, pins Docker base images and adds test-oriented build stages, adds a CI job that runs pytest inside the test image, and updates env templates and docs accordingly. ChangesSettings validation, Docker reproducibility, and documentation cleanup
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant DockerEngine
participant TestContainer
participant Pytest
GitHubActions->>DockerEngine: build image (target=test) -> paperscout:ci
DockerEngine->>TestContainer: run container with env vars, COVERAGE_FILE=/tmp/.coverage
TestContainer->>Pytest: execute pytest tests/ -q --cov=paperscout --cov-fail-under=90
Pytest->>TestContainer: write /tmp/.coverage and exit status
TestContainer->>GitHubActions: return exit status and coverage result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Line 20: The Dockerfile's COPY instruction "COPY tests/ tests/" fails because
tests/ is excluded from the build context; update the build context so Docker
can see tests/ before that COPY runs by removing or negating tests/ from
.dockerignore (or otherwise including tests/ in the build context) so the "COPY
tests/ tests/" line in the Dockerfile succeeds; ensure .dockerignore contains an
explicit exception like "!tests/" or simply remove the tests/ exclusion so the
tests directory is available during docker build.
- Around line 48-55: The test stage inherits runtime's ENTRYPOINT
(["python","-m","paperscout"]) so CI args become arguments to that entrypoint
instead of running pytest; in the Dockerfile test stage (the stage named "test")
clear or override the ENTRYPOINT (e.g. add ENTRYPOINT [] or set ENTRYPOINT to
["python","-m","pytest"]) after the USER paperscout line so running `docker run
... python -m pytest` executes pytest directly rather than being passed to the
paperscout entrypoint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10b261e0-f3db-4982-9cb3-c1adbfee79c4
📒 Files selected for processing (13)
.env.example.github/workflows/ci.ymlCONTRIBUTING.mdDockerfileREADME.mddeploy/SERVER_SETUP.mddocker-compose.ymldocs/handoff.mddocs/onboarding.mdsrc/paperscout/__main__.pysrc/paperscout/config.pysrc/paperscout/db.pytests/conftest.py
💤 Files with no reviewable changes (6)
- src/paperscout/db.py
- tests/conftest.py
- docs/onboarding.md
- src/paperscout/main.py
- src/paperscout/config.py
- README.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
125-133: ⚡ Quick winConsider adding coverage report output for easier debugging.
The test command uses
-qand lacks a--cov-reportflag. If coverage falls below the 90% threshold, developers won't see which modules or lines are uncovered. Adding--cov-report=termwould provide actionable feedback when the job fails.📊 Suggested enhancement
- name: Run tests in container run: | docker run --rm --entrypoint python \ -e _PAPERSCOUT_TESTING=1 \ -e SLACK_BOT_TOKEN=xoxb-ci-placeholder \ -e SLACK_SIGNING_SECRET=ci-placeholder-secret \ -e COVERAGE_FILE=/tmp/.coverage \ paperscout:ci \ - -m pytest tests/ -q --cov=paperscout --cov-fail-under=90 + -m pytest tests/ -q --cov=paperscout --cov-report=term --cov-fail-under=90🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 125 - 133, The CI test step running the containerized pytest invocation uses -m pytest tests/ -q --cov=paperscout --cov-fail-under=90 but omits a coverage report, so update the docker run command invocation to include an explicit coverage report flag (for example add --cov-report=term or --cov-report=term-missing) to the pytest args so failing coverage shows which modules/lines are missing; locate the docker run block that sets environment vars and the pytest command and append the chosen --cov-report option to the pytest flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 125-133: The CI test step running the containerized pytest
invocation uses -m pytest tests/ -q --cov=paperscout --cov-fail-under=90 but
omits a coverage report, so update the docker run command invocation to include
an explicit coverage report flag (for example add --cov-report=term or
--cov-report=term-missing) to the pytest args so failing coverage shows which
modules/lines are missing; locate the docker run block that sets environment
vars and the pytest command and append the chosen --cov-report option to the
pytest flags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 093e0078-65fb-4842-b0dd-62deed0eacb1
📒 Files selected for processing (3)
.github/workflows/ci.ymlCONTRIBUTING.mdDockerfile
✅ Files skipped from review due to trivial changes (1)
- CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
9-9: ⚡ Quick winPin the
uvtool image by digest as well.Line 9 still pulls
ghcr.io/astral-sh/uv:0.8.3by tag, which weakens reproducibility compared with the digest-pinned Python base images.Suggested change
-COPY --from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv +COPY --from=ghcr.io/astral-sh/uv:0.8.3@sha256:<resolved-digest> /uv /usr/local/bin/uv🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 9, Update the COPY source to use the uv image pinned by digest instead of the tag to ensure reproducible builds: replace the current COPY --from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv reference with a digest-pinned reference (e.g., ghcr.io/astral-sh/uv@sha256:<digest>) or point to a named build stage created from that digest; modify the Dockerfile COPY invocation (the COPY --from=... /uv /usr/local/bin/uv line) accordingly and ensure the chosen sha256 digest corresponds to the 0.8.3 release you expect.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 61-62: The Docker command "docker build -t paperscout:test ." is
misleading because it tags a full production build as :test; update the
CONTRIBUTING.md entry so the command either explicitly targets the test stage
(add --target test) like "docker build --target test -t paperscout:test ." or
rename the tag to reflect a production build (e.g., "docker build -t
paperscout:latest ."); pick one approach and make the corresponding replacement
for the line containing the "docker build -t paperscout:test ." command.
---
Nitpick comments:
In `@Dockerfile`:
- Line 9: Update the COPY source to use the uv image pinned by digest instead of
the tag to ensure reproducible builds: replace the current COPY
--from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv reference with a
digest-pinned reference (e.g., ghcr.io/astral-sh/uv@sha256:<digest>) or point to
a named build stage created from that digest; modify the Dockerfile COPY
invocation (the COPY --from=... /uv /usr/local/bin/uv line) accordingly and
ensure the chosen sha256 digest corresponds to the 0.8.3 release you expect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b1d4cbb-3b30-48cc-8634-6869a8860e3f
📒 Files selected for processing (14)
.dockerignore.env.example.github/workflows/ci.ymlCONTRIBUTING.mdDockerfileREADME.mddeploy/SERVER_SETUP.mddocker-compose.ymldocs/handoff.mddocs/onboarding.mdsrc/paperscout/__main__.pysrc/paperscout/config.pysrc/paperscout/db.pytests/conftest.py
💤 Files with no reviewable changes (6)
- src/paperscout/db.py
- src/paperscout/config.py
- docs/onboarding.md
- tests/conftest.py
- README.md
- src/paperscout/main.py
|
@coderabbitai full review again |
|
✅ Actions performedFull review triggered. |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 62-63: Add an explicit empty OPS_ALERT_CHANNEL key to the
.env.example so it is discoverable (follow the same pattern used by
NOTIFICATION_CHANNEL); replace the commented example "# OPS_ALERT_CHANNEL=" with
a bare "OPS_ALERT_CHANNEL=" line in the template to make it clear the key exists
and can be set by operators.
In @.github/workflows/ci.yml:
- Line 120: Replace the unpinned checkout action "uses: actions/checkout@v6"
with a pinned commit SHA for that actions/checkout v6 release and explicitly
disable credential persistence; i.e., change "uses: actions/checkout@v6" to
"uses: actions/checkout@<commit-sha-for-v6>" and add a with block containing
"persist-credentials: false" (and optionally "fetch-depth: 0") so credentials
are not persisted and the action is pinned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43c242eb-a067-4194-b188-0fb772efa80b
📒 Files selected for processing (14)
.dockerignore.env.example.github/workflows/ci.ymlCONTRIBUTING.mdDockerfileREADME.mddeploy/SERVER_SETUP.mddocker-compose.ymldocs/handoff.mddocs/onboarding.mdsrc/paperscout/__main__.pysrc/paperscout/config.pysrc/paperscout/db.pytests/conftest.py
💤 Files with no reviewable changes (6)
- docs/onboarding.md
- README.md
- src/paperscout/db.py
- tests/conftest.py
- src/paperscout/main.py
- src/paperscout/config.py
|
@coderabbitai continue to review |
|
(ミ✿ω✿ミ) ✅ Actions performedFull review triggered. |
Summary
Fail fast on missing Slack credentials at settings load, remove dead configuration, and harden Docker builds for reproducible supply chain.
### Issue 35 — Settings / dead config
SLACK_BOT_TOKENandSLACK_SIGNING_SECRETvia@model_validatoronSettings(raisesConfigurationErrorat import unless_PAPERSCOUT_TESTING=1)__main__.py(validation happens whensettingsis constructed)enable_bulk_openstdfromSettings,.env.example, tests, and docspaperscout.db.poolmodule global.env.examplewith required vs optional credential commentsIssue 36 — Docker reproducibility
python:3.12-slimto digest in builder and runtime stagesuv sync --frozenfromuv.lock(hash-verified deps; no floatingpip install .)test/productionDockerfile targets; compose buildsproductiononlydockerjob: build--target testand run pytest with coverage in-containerCONTRIBUTING.mdanddeploy/SERVER_SETUP.mdDesign notes
_PAPERSCOUT_TESTING=1to allow empty tokens (existingtests/test_config.pycoverage).uv.lock+uv sync --frozenrather thanrequirements.txt+pip --require-hashes(aligned with issue 03 lockfile work).Test plan
uv run pytest tests/ -q --cov=paperscout --cov-fail-under=90uv run pre-commit run --all-filesuv lock --checkdocker build --target test -t paperscout:test .and in-container pytest (CIdockerjob)Links
Summary by CodeRabbit
New Features
Chores
Documentation
Behavior