diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 00000000..b5fb6e28 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,167 @@ +# yaml-language-server: $schema=https://storage.googleapis.com/coderabbit_public_assets/schema.v2.json +# +# CodeRabbit configuration for CiteForge. Keys are taken from the published +# schema above, so an unknown key fails validation rather than being ignored. +# +# Scope of this file: what CodeRabbit reads, and what it should say. It is not a +# gate. The one required status check stays `Required CI` in +# .github/workflows/tests.yml, and `reviews.request_changes_workflow` is left +# false so a review comment never blocks a merge on its own. + +language: en-US + +tone_instructions: >- + Direct and technical. No praise, no restating the diff. Prose avoids em-dashes, + avoids colons inside sentences, and never opens a paragraph with a bolded phrase + standing in for a heading. + +early_access: false + +reviews: + profile: assertive + # The CI workflow is the gate. A review must not be able to block a merge by + # itself, which is what request_changes_workflow would do. + request_changes_workflow: false + high_level_summary: true + review_status: true + collapse_walkthrough: false + changed_files_summary: true + sequence_diagrams: false + assess_linked_issues: true + related_issues: true + related_prs: true + # No commit status, so the branch ruleset keeps exactly one required check. + commit_status: false + poem: false + in_progress_fortune: false + abort_on_close: true + + auto_review: + enabled: true + drafts: false + base_branches: + - main + + # Excluded from review entirely: hash-pinned lock files, generated pipeline + # output, and cached API responses. Every path here is either listed in + # .gitignore or is machine-generated rather than authored. output/ is tracked + # but written by the monthly refresh job, not by hand. + path_filters: + - "!requirements.lock" + - "!requirements-dev.lock" + - "!output/**" + - "!data/api_cache/**" + - "!.venv/**" + - "!build/**" + - "!dist/**" + - "!**/*.egg-info/**" + + # Conventions below are quoted from CLAUDE.md and pyproject.toml, not + # invented. Keep them in sync with those two files. + path_instructions: + - path: "citeforge/**/*.py" + instructions: >- + Type annotations on every public function and method, enforced by mypy with + disallow_untyped_defs and check_untyped_defs (pyproject.toml [tool.mypy]). + Formatting and linting are ruff at line-length 120 with rules + E/F/W/I/N/UP/B/C4/SIM/RUF/S; the repository does not run ruff format, so do + not propose reformatting. + + Flag any violation of the invariants CLAUDE.md states: + thresholds, API endpoints, trust order, rate limits and compound word + dictionaries live in citeforge/config.py and are never hardcoded elsewhere + ("Config-driven"); the pipeline produces byte-identical output across + consecutive cache-hit runs, so directory and file iteration uses sorted() and + nothing that affects output is randomized ("Determinism"); DOI comparison goes + through _norm_doi() from citeforge/id_utils.py and is always paired with a + title_similarity >= 0.55 check; an orphan .bib file is never deleted without + verifying it as a duplicate at title_similarity >= 0.95. + + FUSED_COMPOUND_WORDS and ABBREVIATED_VENUE_MAP must contain no em-dash and no + accented characters, because the serializer strips them. + + Comments and log messages are English. Prose in comments avoids em-dashes, + avoids colons inside sentences, and does not open a paragraph with a bolded + pseudo-heading. Do not accept new TODO markers. + - path: "citeforge/merge_utils.py" + instructions: >- + Trust hierarchy. merge_with_policy() merges fields from 13 ranked sources with + override rules for DOI (published beats preprint), journal (never downgrade to + a preprint), title (prefer the longer), pages (reject invalid) and booktitle + (upgrade a generic series to the conference name). A change to the ranking or + to any override rule changes published bibliographic output, so it needs a + test that pins the new behaviour. + - path: "citeforge/pipeline/**/*.py" + instructions: >- + CLAUDE.md "Three-Way Fix Pattern" is the critical invariant here. A fix to + entry types, titles or booktitles must be applied in all three places, or the + output oscillates between consecutive runs: _fixup_bib_entry() on load, the + existing-file fixup in process_article() before enrichment, and the Phase 4 + post-merge step. The consolidated helpers _fix_title_text() and + _apply_booktitle_fixups() are called from all three. Flag a new text or type + fix added to fewer than all three call sites. + - path: "citeforge/clients/**/*.py" + instructions: >- + Scholarly API clients. API keys are read from keys/, which is gitignored, and + must never be inlined, logged, or written into output. This repository is + public, so a raw API response must not be committed to the tree; the monthly + refresh encrypts its cache before pushing. Check that new HTTP calls go through + citeforge/http_utils.py rather than calling requests directly, so retry and + rate-limit handling stays in one place. + - path: "tests/**/*.py" + instructions: >- + pytest with plain assert statements, fixtures and @pytest.mark.parametrize. + Shared fixtures live in tests/conftest.py and tests/fixtures.py. HTTP is mocked + with monkeypatch and a unit test never makes a real API call; integration tests + that need keys auto-skip when the keys are absent. Flag a test that is skipped + or xfailed instead of fixed, and flag an assertion that cannot fail. Coverage + has a floor of 68 percent enforced by --cov-fail-under in the CI test job, so a + change that drops coverage below it fails the build. + + CLAUDE.md forbids adding automated audit modules; issues are fixed in pipeline + code or by direct .bib edits. + - path: ".github/workflows/*.yml" + instructions: >- + Actions are pinned by commit SHA with the version in a trailing comment, never + by tag or branch. Every job carries timeout-minutes. Any job added must also be + added to the `needs` list of the `CI` job named `Required CI` in tests.yml, + which is the single aggregate status check branch protection requires; a job + outside that list is unprotected. This repository stays on GitHub Actions and + must not gain a .circleci directory. + - path: "**/*.md" + instructions: >- + Prose avoids em-dashes, avoids colons inside sentences, and never opens a + paragraph with a bolded phrase used as a heading. CLAUDE.md owns the + architecture description and the Three-Way Fix Pattern; link to it rather than + restating it. Do not commit counts or statistics that drift with normal + pipeline runs. + + tools: + ruff: + enabled: true + actionlint: + enabled: true + gitleaks: + enabled: true + markdownlint: + enabled: true + shellcheck: + enabled: true + yamllint: + enabled: true + # Off deliberately: this repository has no JavaScript, TypeScript, Docker or + # Helm surface, so these only produce noise. + eslint: + enabled: false + biome: + enabled: false + hadolint: + enabled: false + +chat: + auto_reply: true + +knowledge_base: + opt_out: false + learnings: + scope: local diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8eaf92ae..05124b91 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -6,12 +6,26 @@ on: pull_request: branches: [ main, master ] +permissions: + contents: read + +concurrency: + group: tests-${{ github.ref }} + # Supersede stale PR runs, but never cancel an in-progress run on main: a + # rapid second merge must not leave a commit on the default branch with + # cancelled (never-completed) required checks. + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: lint: + name: Lint and types runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 @@ -22,7 +36,10 @@ jobs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.cache/pip - key: ${{ runner.os }}-pip-lint-${{ hashFiles('pyproject.toml') }} + # The lock file is what gets installed, so it belongs in the key. + # Keying on pyproject.toml alone leaves the cache stale whenever the + # lock is regenerated without a pyproject change. + key: ${{ runner.os }}-pip-lint-${{ hashFiles('pyproject.toml', 'requirements-dev.lock') }} restore-keys: | ${{ runner.os }}-pip- @@ -39,7 +56,9 @@ jobs: run: mypy citeforge/ main.py test: + name: Test (py${{ matrix.python-version }}) runs-on: ubuntu-latest + timeout-minutes: 30 strategy: max-parallel: 1 fail-fast: true @@ -48,6 +67,8 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 @@ -58,7 +79,7 @@ jobs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ matrix.python-version }}-${{ hashFiles('pyproject.toml') }} + key: ${{ runner.os }}-pip-${{ matrix.python-version }}-${{ hashFiles('pyproject.toml', 'requirements-dev.lock') }} restore-keys: | ${{ runner.os }}-pip-${{ matrix.python-version }}- ${{ runner.os }}-pip- @@ -103,3 +124,40 @@ jobs: path: | .pytest_cache/ retention-days: 7 + + # Single aggregate gate. Branch protection requires this one check, so a job + # added below is covered as soon as it joins the `needs` list, and a renamed + # job cannot silently stop being required. + CI: + name: Required CI + if: always() + needs: + - lint + - test + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Check job results + env: + # The whole `needs` context as JSON, rather than one env var per job. + # Enumerating them meant a job added to `needs` above was still not + # checked here unless this script was edited too, which defeats the + # point of the gate. Iterating means joining `needs` is sufficient. + NEEDS: ${{ toJSON(needs) }} + run: | + # A required check must either succeed or skip on purpose via its own + # `if:` guard. No job in this workflow carries an `if:`, so a `skipped` + # result means a dependency never ran and is treated as a failure. + if [ "$(jq 'length' <<<"$NEEDS")" -eq 0 ]; then + echo "FAIL: the gate has no dependencies, so it would pass vacuously" + exit 1 + fi + FAILED=0 + while IFS=$'\t' read -r job result; do + case "$result" in + success) echo "OK: $job ($result)" ;; + skipped) echo "FAIL: $job was skipped; required checks must run"; FAILED=1 ;; + *) echo "FAIL: $job ($result)"; FAILED=1 ;; + esac + done < <(jq -r 'to_entries[] | "\(.key)\t\(.value.result)"' <<<"$NEEDS") + exit $FAILED diff --git a/.pr_agent.toml b/.pr_agent.toml new file mode 100644 index 00000000..8a596c95 --- /dev/null +++ b/.pr_agent.toml @@ -0,0 +1,76 @@ +# Qodo Merge (PR-Agent) repository configuration. +# +# Read from the root of the repository's DEFAULT branch, so a change here only +# takes effect once merged. Only keys that override a default are listed; every +# key below exists in pr_agent/settings/configuration.toml or +# pr_agent/settings/ignore.toml upstream. +# +# This is advice, not a gate. The single required status check stays +# `Required CI` in .github/workflows/tests.yml. + +[config] +response_language = "en-US" + +# Excluded from analysis: hash-pinned lock files, generated pipeline output, and +# cached API responses. Each path is either listed in .gitignore or is +# machine-generated rather than authored. output/ is tracked but written by the +# monthly refresh job, not by hand. +[ignore] +glob = [ + 'requirements.lock', + 'requirements-dev.lock', + 'output/**', + 'data/api_cache/**', + '.venv/**', + 'build/**', + 'dist/**', + '**/*.egg-info/**', +] +# No `regex` entry. `.*\.key$` excluded exactly the files a leaked credential +# would live in, in a PUBLIC repository whose own instructions two sections +# below say to treat a committed API key as a defect. An earlier commit removed +# the equivalent glob exclusions from .coderabbit.yaml and greptile.json but +# missed this one, which is a different key in the same file. + +[pr_reviewer] +require_tests_review = true +require_security_review = true +require_estimate_effort_to_review = false +require_can_be_split_review = false +require_todo_scan = true +persistent_comment = true +num_max_findings = 5 +extra_instructions = """\ +Direct and technical. No praise, no restating the diff. Prose avoids em-dashes, avoids colons inside sentences, and never opens a paragraph with a bolded phrase standing in for a heading. + +Do not repeat what CI already reports. ruff lints, mypy typechecks, and a coverage floor of 68 percent is enforced by --cov-fail-under in the test job. Report only defects a maintainer would act on. + +This repository is public. Treat anything that could commit an API key, a raw API response, or a contributor's private data as a defect rather than a nit. Keys are read from the gitignored keys/ directory and must never be inlined, logged or written into output. + +Python conventions from CLAUDE.md and pyproject.toml: type annotations on every public function and method (mypy runs with disallow_untyped_defs and check_untyped_defs), ruff at line-length 120 with rules E/F/W/I/N/UP/B/C4/SIM/RUF/S. The repository does not run ruff format, so do not propose reformatting. + +Invariants a change must not break: +- Config-driven. Thresholds, API endpoints, trust order, rate limits and compound word dictionaries live in citeforge/config.py and are never hardcoded elsewhere. +- Determinism. The pipeline produces byte-identical output across consecutive cache-hit runs. Directory and file iteration uses sorted(), and nothing that affects output is randomized. +- DOI comparison goes through _norm_doi() from citeforge/id_utils.py and is always paired with a title_similarity >= 0.55 check. +- An orphan .bib file is never deleted without verifying it as a duplicate at title_similarity >= 0.95. +- FUSED_COMPOUND_WORDS and ABBREVIATED_VENUE_MAP contain no em-dash and no accented characters, because the serializer strips them. + +Under citeforge/pipeline/, the Three-Way Fix Pattern is critical. A fix to entry types, titles or booktitles must be applied in all three places or the output oscillates between consecutive runs: _fixup_bib_entry() on load, the existing-file fixup in process_article() before enrichment, and the Phase 4 post-merge step. Flag a new text or type fix added to fewer than all three call sites. + +Under citeforge/merge_utils.py, merge_with_policy() ranks 13 sources with override rules for DOI, journal, title, pages and booktitle. A change to the ranking or to an override rule changes published bibliographic output and needs a test that pins the new behaviour. + +Under tests/, HTTP is mocked with monkeypatch and a unit test never makes a real API call; integration tests that need keys auto-skip when the keys are absent. Flag a test skipped or xfailed instead of fixed, and flag an assertion that cannot fail. + +In .github/workflows/, every action is pinned by commit SHA with a trailing version comment and every job carries timeout-minutes. A job added to tests.yml must also be added to the needs list of the CI job named Required CI, or it is not covered by branch protection. This repository stays on GitHub Actions and must not gain a .circleci directory. +""" + +[pr_code_suggestions] +commitable_code_suggestions = false +focus_only_on_problems = true +persistent_comment = true +extra_instructions = """\ +Suggest a change only when the current code is wrong, unsafe, or breaks a stated invariant. Do not suggest stylistic rewrites, do not suggest adding abstractions for hypothetical future needs, and do not suggest a dependency where a few lines of the standard library would do. + +Never suggest adding an automated audit module; CLAUDE.md states that issues are fixed in pipeline code or by direct .bib edits. Never suggest hardcoding a threshold, endpoint or trust rank outside citeforge/config.py. +""" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..2cf294d3 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,66 @@ +# Local pre-commit hooks. These mirror the CI quality gates in +# .github/workflows/tests.yml rather than adding new ones, so a clean local run +# predicts a clean CI run. Tool versions are pinned to the same releases that +# requirements-dev.lock installs, so local and CI cannot drift. +# +# Install with: pip install pre-commit && pre-commit install + +exclude: | + (?x)^( + output/.* + | data/api_cache/.* + | requirements\.lock + | requirements-dev\.lock + )$ + +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v6.0.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: mixed-line-ending + args: [--fix=lf] + - id: check-yaml + - id: check-toml + - id: check-merge-conflict + - id: check-case-conflict + - id: check-added-large-files + - id: debug-statements + # This repository is public and keys/ is gitignored. This is the last + # line of defence if a key file is ever force-added. + - id: detect-private-key + + # Matches requirements-dev.lock ruff==0.15.20 and the CI step + # `ruff check citeforge/ tests/ main.py`. ruff-format is deliberately absent: + # the repository configures ruff for linting only (pyproject.toml [tool.ruff] + # sets line-length and a lint rule set, with no formatter section), and CI + # never runs `ruff format --check`. + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.15.20 + hooks: + - id: ruff-check + args: [--fix] + + # mypy runs from the project environment rather than from + # pre-commit/mirrors-mypy, which would install it into an isolated virtualenv. + # That isolation is the wrong shape here: rapidfuzz and unidecode would be + # absent, ignore_missing_imports would type them as Any, and warn_return_any + # would then report errors in citeforge/text_utils.py that CI does not see + # because CI installs the full lock. Reusing the local environment keeps the + # hook and the CI step (`mypy citeforge/ main.py`) on identical inputs, and + # the version stays pinned by requirements-dev.lock (mypy==2.1.0) instead of + # by a second rev that could drift from it. + # + # Requires the dev extras to be installed, which CLAUDE.md already documents + # as the setup step: pip install -e .[dev] + - repo: local + hooks: + - id: mypy + name: mypy + entry: mypy + language: system + types: [python] + args: [citeforge/, main.py] + pass_filenames: false + require_serial: true diff --git a/greptile.json b/greptile.json new file mode 100644 index 00000000..cadcde9a --- /dev/null +++ b/greptile.json @@ -0,0 +1,72 @@ +{ + "commentTypes": ["logic", "syntax"], + "strictness": 2, + "triggerOnUpdates": true, + "triggerOnDrafts": false, + "shouldUpdateDescription": false, + "statusCheck": false, + "instructions": "Direct and technical. No praise and no restating the diff. Prose avoids em-dashes, avoids colons inside sentences, and never opens a paragraph with a bolded phrase standing in for a heading. Report a finding only when it is a defect a maintainer would act on; the CI workflow already covers linting, typechecking and a coverage floor, so do not repeat what ruff, mypy or actionlint would say. CI is the gate, this review is advice: never request changes purely on style. This repository is public, so treat anything that could leak an API key, a raw API response, or a contributor's private data as a defect rather than a nit.", + "ignorePatterns": "requirements.lock\nrequirements-dev.lock\noutput/**\ndata/api_cache/**\n.venv/**\nbuild/**\ndist/**\n**/*.egg-info/**", + "ignoreKeywords": "linter\nruff format\ngreptile-ignore", + "customContext": { + "rules": [ + { + "scope": ["citeforge/**/*.py"], + "rule": "Type annotations on every public function and method, enforced by mypy with disallow_untyped_defs and check_untyped_defs. Linting is ruff at line-length 120 with rules E/F/W/I/N/UP/B/C4/SIM/RUF/S. The repository does not run ruff format, so do not propose reformatting." + }, + { + "scope": ["citeforge/**/*.py"], + "rule": "Config-driven. Thresholds, API endpoints, trust order, rate limits and compound word dictionaries live in citeforge/config.py and are never hardcoded elsewhere. Flag a magic number or a literal endpoint introduced outside that module." + }, + { + "scope": ["citeforge/**/*.py"], + "rule": "Determinism. The pipeline produces byte-identical output across consecutive cache-hit runs. Directory and file iteration uses sorted(), and nothing that affects output is randomized. Flag an unsorted os.listdir, glob or set iteration whose order reaches the output, and flag any use of random, time or uuid in an output-affecting path." + }, + { + "scope": ["citeforge/**/*.py"], + "rule": "DOI comparison goes through _norm_doi() from citeforge/id_utils.py and is always paired with a title_similarity >= 0.55 check. An orphan .bib file is never deleted without first verifying it as a duplicate at title_similarity >= 0.95. FUSED_COMPOUND_WORDS and ABBREVIATED_VENUE_MAP must contain no em-dash and no accented characters, because the serializer strips them." + }, + { + "scope": ["citeforge/pipeline/**/*.py"], + "rule": "Three-Way Fix Pattern, the critical invariant. A fix to entry types, titles or booktitles must be applied in all three places or the output oscillates between consecutive runs: _fixup_bib_entry() on load, the existing-file fixup in process_article() before enrichment, and the Phase 4 post-merge step. The consolidated helpers _fix_title_text() and _apply_booktitle_fixups() are called from all three. Flag a new text or type fix added to fewer than all three call sites." + }, + { + "scope": ["citeforge/merge_utils.py"], + "rule": "Trust hierarchy. merge_with_policy() merges fields from 13 ranked sources with override rules for DOI (published beats preprint), journal (never downgrade to a preprint), title (prefer the longer), pages (reject invalid) and booktitle (upgrade a generic series to the conference name). A change to the ranking or to any override rule changes published bibliographic output and needs a test that pins the new behaviour." + }, + { + "scope": ["citeforge/clients/**/*.py"], + "rule": "Scholarly API clients. Keys are read from the gitignored keys/ directory and must never be inlined, logged or written into output. This repository is public, so a raw API response must not be committed. New HTTP calls go through citeforge/http_utils.py rather than calling requests directly, so retry and rate-limit handling stays in one place." + }, + { + "scope": ["tests/**/*.py"], + "rule": "pytest with plain assert statements, fixtures and parametrize. Shared fixtures live in tests/conftest.py and tests/fixtures.py. HTTP is mocked with monkeypatch and a unit test never makes a real API call; integration tests that need keys auto-skip when the keys are absent. Coverage has a floor of 68 percent enforced by --cov-fail-under in CI. Flag a test skipped or xfailed instead of fixed, and flag an assertion that cannot fail. Do not suggest adding automated audit modules; CLAUDE.md forbids them." + }, + { + "scope": [".github/workflows/*.yml"], + "rule": "Actions are pinned by commit SHA with the version in a trailing comment, never by tag or branch, and every job carries timeout-minutes. Any job added must also be added to the needs list of the CI job named Required CI in tests.yml, because that aggregate is the single status check branch protection requires. A job outside that list is unprotected. This repository stays on GitHub Actions and must not gain a .circleci directory." + }, + { + "scope": ["**/*.md"], + "rule": "Prose avoids em-dashes, avoids colons inside sentences, and never opens a paragraph with a bolded pseudo-heading. Do not commit TODO markers or counts that drift with normal pipeline runs." + } + ], + "files": [ + { + "scope": ["citeforge/**", "main.py", "tests/**"], + "path": "CLAUDE.md", + "description": "Repository conventions and invariants. The Architecture section owns the four-phase pipeline and the post-run ordering, the Three-Way Fix Pattern section owns the oscillation invariant, and Key Conventions owns config-driven values, determinism, DOI normalization and orphan safety." + }, + { + "scope": ["**"], + "path": "README.md", + "description": "Project overview, the trust-based aggregation model, and the scholarly data sources the clients talk to." + }, + { + "scope": ["citeforge/**", "main.py"], + "path": "pyproject.toml", + "description": "Authoritative tool configuration. [tool.ruff] sets line-length 120 and the selected rule set with its ignores, [tool.mypy] sets the strictness flags, and [tool.pytest.ini_options] sets the test discovery paths." + } + ] + } +}