Skip to content

feat(milestone-1): phase 5 — deprecations, docs hygiene, security stubs#776

Closed
derks wants to merge 28 commits into
mainfrom
modernization-phase-5-pr
Closed

feat(milestone-1): phase 5 — deprecations, docs hygiene, security stubs#776
derks wants to merge 28 commits into
mainfrom
modernization-phase-5-pr

Conversation

@derks
Copy link
Copy Markdown
Member

@derks derks commented May 8, 2026

Summary

Phase 5 of the Cement 3 modernization milestone: deprecations cleanup,
docs build hygiene, and planning-side security-tooling stubs. Six
plans executed across six waves; verification passed 12/12 must-haves;
post-verification code review surfaced two real regressions and four
sister findings, all fixed before this PR. All project gates (ruff,
mypy, audit-public-api, 100% test coverage, zero-warnings docs build)
green.

Changes

Deprecations (Plan 05-01, 05-03)

  • [core.deprecations] Pin every entry in DEPRECATIONS to a concrete
    removal version (Cement v3.2.0). Drop the trailing period from the
    3.0.10-1 value so the runtime warning no longer renders as ..
    before the GitBook URL (CR-02 from review).
  • [ext.logging] Tighten removal-version language in the set_level
    and fatal deprecation docstrings — Cement v3.2.0 named explicitly,
    with double-backtick RST for inline code.
  • [ext.smtp] Document the send() bool-return → senderrs-dict
    contract change as a .. deprecated:: 3.0.16 admonition so autodoc
    renders the deprecation in HTML.
  • DEPRECATIONS.md (new, repo root): in-repo touchpoint with a 5-element
    block per ID (Surface / Since / Removal / migration prose / GitBook
    back-link). Anchors use the dotted form (#3.0.8-1) so the link
    emitted by cement.core.deprecations.deprecate() lands on the same
    GitBook fragment.

Docs build hygiene (Plan 05-02, 05-04, +follow-ups)

  • [sphinx] Resolve all five Sphinx warnings under
    pdm run sphinx-build: drop unsupported logo theme option, rename
    display_versionversion_selector (sphinx_rtd_theme 3.0+),
    string-quote a list[str] autodoc return annotation in
    core.interface, reflow the cmd() Returns docstring so the
    inline-literal stays single-line, delete the unused
    docs/source/api/index.rst orphan.
  • [make] Wire -W (warnings-as-errors) into make docs and switch
    the recipe from ; to && chaining (CR-01 from review). Without
    &&, the recipe's exit code came from the trailing cd .. (always
    0), so the zero-warnings gate this phase was meant to install was
    non-functional — it now genuinely fails the build on any new warning.
  • [docs] Expose cement.core.deprecations in the Sphinx API
    reference (was missing entirely). Adds
    docs/source/api/core/deprecations.rst + toctree entry, plus
    minimal docstrings on the module, DEPRECATIONS dict, and
    deprecate() so the autodoc page renders meaningful content rather
    than an empty stub. Closes a real DEPREC-02 gap surfaced during
    review: the module that emits every runtime DeprecationWarning was
    internally imported by four sites but never had a :mod: page.
  • [docs] Fix stderrorstderr typo in
    cement.utils.shell:cmd() and exec_cmd() Returns docstrings (4
    occurrences). Example variables and runtime code already used the
    correct name; only the prose mismatched. Sister fix to Plan 05-02's
    inline-literal RST work, which reflowed the same prose without
    catching the typo.

Public-facing docs (Plan 05-05, +follow-up)

  • README.md — drop both Travis-CI surfaces (badge + More Information
    link); CI moved to GitHub Actions long ago.
  • .github/CONTRIBUTING.md — align with Conventional Commits +
    atomic-per-concern discipline; reference make commit; back-link to
    ../CLAUDE.md. Remove the orphaned [Commit Guidelines]:
    reference-link definition that lost its body callsites in the Plan
    05-05 rewrite (WR-01 follow-up).

Planning-side security stubs (Plan 05-06)

  • .planning/REQUIREMENTS.md — expand SECv2-01..03 (pip-audit,
    bandit, SAST/CodeQL) with phase-shaped scope blocks (Tool, Make
    target, CI placement, Config, New dev-dep, Exit behavior) so the
    future security-tooling milestone has hand-off-ready starting points.
  • (.planning/codebase/CONVENTIONS.md PEP 604/585 refresh is
    intentionally not part of this PR — it's a transient planning
    artifact and lands via the post-merge archive commit.)

CHANGELOG hygiene

  • Earlier [docs] Wire api/index into top-level toctree entry was
    factually wrong — Plan 05-02 actually deleted the orphan. Replaced
    with text matching the actual repo state (docs/source/index.rst
    references api/core/index, api/utils/index, api/ext/index
    directly).

Acceptance status

Req ID Description Status
DEPREC-01 Concrete removal version on every deprecation
DEPREC-02 In-repo deprecation touchpoint ✓ (DEPRECATIONS.md narrative + Sphinx API page for cement.core.deprecations, with documented Sphinx-vs-GitBook split)
DEPREC-03 Existing tests still pass under tightened messages
DOCS-01 make docs is zero-warnings (genuinely enforced via -W + &&)
DOCS-02 README + CONTRIBUTING accurate against 3.0.16
DOCS-04 No stale Travis / Python 3.9 / setup.py references in cement/
SEC-01 pip-audit phase-shaped scope captured ✓ (planning)
SEC-02 bandit phase-shaped scope captured ✓ (planning)
SEC-03 SAST/CodeQL phase-shaped scope captured ✓ (planning)

VERIFICATION.md status: passed. 12/12 must-haves verified (1 via the
documented CONTEXT.md D-05/D-06 override that reinterprets DEPREC-02's
medium from docs/source/deprecations.rst to repo-root
DEPRECATIONS.md — Sphinx is strictly API reference per
docs/source/index.rst; developer narrative lives at builtoncement.com).

Files touched

Code:

  • cement/core/deprecations.py
  • cement/core/interface.py
  • cement/ext/ext_logging.py
  • cement/ext/ext_smtp.py
  • cement/utils/shell.py
  • docs/source/conf.py
  • docs/source/api/core/deprecations.rst (new)
  • docs/source/api/core/index.rst (toctree entry)
  • docs/source/api/index.rst (deleted — orphan)

Build / docs / contributor surfaces:

  • Makefile
  • README.md
  • .github/CONTRIBUTING.md
  • DEPRECATIONS.md (new)
  • CHANGELOG.md

Structural planning state:

  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md

Test plan

  • make comply — ruff + mypy clean
  • make audit-public-api — public surface byte-identical
  • make test — 320 passing, 100% coverage held
  • make docs — zero-warnings build under -W (clean baseline)
  • Inject a deliberate Sphinx warning, confirm make docs now exits
    non-zero (verifies the && fix bites where the prior ; form
    silently passed)
  • Trigger cement.core.deprecations.deprecate('3.0.10-1'), confirm
    the runtime warning has a single period before See: ... (no ..)
  • Sphinx HTML render of SMTPMailHandler.send() shows the
    .. deprecated:: 3.0.16 admonition correctly
  • docs/build/api/core/deprecations.html renders with both
    DEPRECATIONS and deprecate documented (not an empty stub)
  • docs/build/api/utils/shell.html renders cmd() and
    exec_cmd() with stderr (no stderror) in the Returns prose

🤖 Generated with Claude Code

derks and others added 28 commits May 7, 2026 19:27
Maintainer triaged the github.com/datafolklabs/cement backlog directly
(80 open issues, 14-year tail) outside the GSD workflow rather than
producing CONTEXT/PLAN/VERIFICATION artifacts. Records the closure
comment template used and per-requirement disposition in 04-NOTE.md;
flips TRIAGE-01..04 to Closed and updates ROADMAP/STATE accordingly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Land 6 PLAN.md files covering the 14-commit D-16 sequence from
05-CONTEXT.md, organized into 6 linearized waves (CHANGELOG.md file
ownership forces serialization, matching Phase 2 precedent):

- 05-01-PLAN.md (Wave 1): tighten DEPRECATIONS registry + adjacent
  docstring sweep across ext_logging.py and ext_smtp.py — D-16 1/2/3
- 05-02-PLAN.md (Wave 2): resolve all 4 sphinx warnings (conf.py
  duplicate dict, api/index.rst orphan, interface.py:102 string-quote,
  shell.py cmd() inline-literal RST) — D-16 5/6/7/8
- 05-03-PLAN.md (Wave 3): add top-level DEPRECATIONS.md mirroring
  GitBook narrative with 4 H2 blocks — D-16 4
- 05-04-PLAN.md (Wave 4): wire -W into make docs + AUDIT POINT
  comment — D-16 9
- 05-05-PLAN.md (Wave 5): drop Travis surfaces from README + align
  CONTRIBUTING with Conventional Commits + DOCS-04 fix-on-find sweep
  (RESEARCH.md A5 pre-verified 0 hits) — D-16 10/11/optional 12
- 05-06-PLAN.md (Wave 6): sync CONVENTIONS.md ruff target-version
  to py310 + expand SECv2-01..03 with phase-shaped scope notes
  (planning-artifact, NO CHANGELOG entries) — D-16 13/14

Plans are written prompts: every task carries <read_first> with
file:line anchors, <action> with verbatim BEFORE/AFTER text from
RESEARCH.md Code Examples 1-10, and <acceptance_criteria> with
grep-verifiable conditions encoded against the bucket-per-commit
table from RESEARCH.md Pitfall 7. The 8 RESEARCH.md pitfalls (esp.
Pitfall 2 conf.py duplicate dict, Pitfall 5 -W flag ordering,
Pitfall 6 CONVENTIONS already-mostly-done, Pitfall 7 planning-
artifact filter for commits 13/14, Pitfall 8 dotted GitBook
anchors) are encoded as task-level guards.

Each plan includes a <threat_model> block per ASVS L1; the threat
surface is narrow (no new code paths) and dispositions are mostly
accept/mitigate against information-disclosure (stale links, anchor
format) and tampering (config edits, planning-intel drift).

ROADMAP.md updated: Phase 5 Plans block populated with 6 plans
across 6 waves; progress table updated to 0/6.

Validation: gsd-sdk frontmatter validate + verify plan-structure
all return valid:true with zero errors and zero warnings on all 6
plans.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pin 3.0.10-1 (FATAL logging) and 3.0.16-1 (SMTPMailHandler.send bool
return) to v3.2.0 in DEPRECATIONS dict. Aligns with the existing
3.0.8-1 / 3.0.8-2 policy. Removal target is the next breakage-allowed
minor; downstream apps that have ignored the warning since 3.0.10
have had ample time.

DEPREC-01 acceptance criterion #5: every entry in
cement/core/deprecations.py:DEPRECATIONS now names a removal version.
DEPREC-03 already-satisfied — existing tests assert by ID, not by
message text.
…ocstrings

Pin set_level() and fatal() docstrings to v3.2.0 (matches the
3.0.10-1 DEPRECATIONS entry tightened in the prior commit). Also
fix the "us"/"use" typo and switch single-backtick CRITICAL /
critical() to RST inline-literal double-backtick form so autodoc
renders them as code, not emphasis.

DEPREC-01 acceptance carried at the docstring layer for the
load-bearing user-visible surface.
Insert a Sphinx .. deprecated:: 3.0.16 admonition into the send()
docstring between Returns and Example. Pairs with the runtime
deprecate('3.0.16-1') call at line 189-190 (unchanged) so the
deprecation is visible at both runtime AND in autodoc HTML.

Also fix a zero-space-after-colon RST formatting bug
(bool:``True`` -> bool: ``True``) in the same Returns block.

DEPREC-01 acceptance carried at the docstring layer for the
SMTPMailHandler.send return-type deprecation.
sphinx_rtd_theme 3.1.0 rejects 'logo' as an unsupported option.
The duplicate html_theme_options dict at conf.py:51-53 silently
overwrote the active dict at lines 19-33; deleting it (3 lines)
restores the toc-config (collapse_navigation, navigation_depth,
titles_only, etc.) AND drops the warning.

DOCS-01 progress: 1 of 4 known sphinx warnings resolved.
docs/source/api/index.rst was an orphan duplicate of the top-level
docs/source/index.rst toctree (both referenced api/core/index,
api/utils/index, api/ext/index). Sphinx flagged the orphan with
"document isn't included in any toctree". Deleting the orphan keeps
the top-level toctree as the single source of truth.

Pre-flight: no external references to api/index.html in .planning/,
docs/, or README.md.

DOCS-01 progress: 2 of 4 known sphinx warnings resolved.
The InterfaceManager.list method-name shadows the builtin, so
autodoc's inspect.signature() introspection tries to subscript the
method object (self.list, which has type function) instead of
builtins.list, producing:

  WARNING: Failed to get a method signature for
  cement.core.interface.InterfaceManager.list:
  'function' object is not subscriptable

String-quoting the return annotation defers evaluation past the
method-name binding. The local fix preserves Phase 3 D-08
(from __future__ import annotations removal stays in effect
file-wide).

Inline `# autodoc:` comment is grep-able for future cleanup. The
sibling files (hook.py, handler.py, extension.py) use a different
pattern (import builtins + builtins.list[T]) — preserved as a
future consistency-cleanup candidate per RESEARCH.md Open
Question 1.

audit-public-api is annotation-blind (verified) — no baseline
rebase needed.

DOCS-01 progress: 3 of 4 known sphinx warnings resolved.
sphinx_rtd_theme 3.0+ renamed the `display_version` theme option to
`version_selector`. The active html_theme_options dict at conf.py:19
still carried the old name; the warning was masked while the
duplicate-dict overwrote the active config and only surfaced after
Plan 05-02 Task 1 deleted the duplicate.

This is a Rule 1/3 deviation from the Plan 05-02 task list: the
4 known sphinx warnings the plan was scoped against did NOT include
this one (it became visible only after the Task 1 delete). Fixing
it here is required to meet Task 4's phase-level gate precondition
(`sphinx-build ... | grep -cE '(WARNING|ERROR):' returns 0`) so
Plan 04 can flip `make docs` to use `-W`.

DOCS-01 progress: bonus warning resolved en route to the original 4.
The actual bug was an inline literal that spanned a line break in
the source — `\`\`(stdout, stderror,⏎ return_code)\`\`` — RST inline
literals must close on the same line they open. Reflowing the
prose so the parenthesized literal stays on one line clears the
docutils warning.

The slash separator between `\`\`text=True\`\` /\`\`encoding=...\`\``
was a separate concern called out in Plan 05-02 Pitfall 3. Pitfall
3's prescribed `or` swap alone did NOT clear the warning; the
line-spanning literal above it was the load-bearing fix. The slash
is also replaced with `or` in the same reflow for consistency with
the plan's intent.

`exec_cmd()` (lines 73-91) still carries the same slash form per
plan instructions (D-08 #4 names only `cmd()`); no warning fires
there because its docstring keeps `\`\`(stdout, stderror,
return_code)\`\`` on a single line — confirming the line-spanning
literal was the actual root cause.

DOCS-01 progress: 4 of 4 originally-known sphinx warnings resolved.
After this commit + the version_selector rename, sphinx-build
emits 0 warnings; Plan 04 (D-16 step 9) can flip `make docs` to
use `-W`.
Mirror the canonical GitBook deprecation narrative at
docs.builtoncement.com/release-information/deprecations into a
top-level repo-root markdown file (sibling of README.md,
CHANGELOG.md, CONTRIBUTORS.md). Anchors use the dotted-version
format (#3.0.8-1, NOT #3-0-8-1) per the runtime URL emitted by
cement.core.deprecations.deprecate().

Per CONTEXT.md D-05/D-06: Sphinx is strictly API reference per
docs/source/index.rst — free-form developer narrative lives on
GitBook. The literal REQUIREMENTS.md DEPREC-02 wording ("a dedicated
docs/source/deprecations.rst page") was authored without this
docs-split context and is reinterpreted, not followed verbatim.

DEPREC-02 acceptance: in-repo touchpoint with one H2 section per
registry ID; per-block format = surface, since, removal, migration
prose with code example, GitBook back-link.
Flip the docs target from `sphinx-build ./source ./build` to
`sphinx-build -W ./source ./build` so future warnings fail-fast.
Plan 02 (D-16 steps 5-8) resolved all 4 known warnings — this commit
makes that posture enforced, not aspirational.

AUDIT POINT comment matches the established convention: Phase 1 D-08
(ruff family AUDIT POINT in pyproject.toml), Phase 2 D-10/D-11
(coverage gate AUDIT POINT), Phase 3 D-06 (UP+FA family AUDIT POINT).
The grep `grep -nE 'AUDIT POINT' Makefile pyproject.toml` now lists
every deliberate-configuration site across the repo.

DOCS-01 acceptance: make docs exits 0 with -W enabled — zero
warnings, no broken cross-references.

Note: -W is local-only this phase. CI integration of `make docs` is
deferred to a future milestone (CONTEXT.md <deferred>).
Travis CI was retired in Phase 1 (.travis.yml deleted, CI moved
to GitHub Actions). The README still carried two stale references:
the build-status badge at line 5 (app.travis-ci.com) and a More
Information list-item link at line 60 (travis-ci.org).

Both deleted. The remaining badges and link list are unchanged.

DOCS-02 acceptance: README is accurate against the 3.0.16 release.
Replace "A single commit per issue" with the project's actual
current discipline: atomic per concern + Conventional Commits +
78-char wrap + `make commit` (cz). Add a Commit Conventions
subsection with subject/body/type/authoring rules and a back-link
to CLAUDE.md as the canonical commit-shape doc.

The "A single commit per issue" guidance was the inverse of how
Phases 1, 2, and 3 actually shipped (84 atomic commits in Phase 3
alone) and would have given external contributors the wrong
forward-looking signal.

Existing reference-link block (PEP8, issue tracker, etc.) is
preserved; [Conventional Commits] target appended to the block.

DOCS-02 acceptance: CONTRIBUTING aligns with the 3.0.16 release
discipline.
Expand SECv2-01 (pip-audit), SECv2-02 (bandit), SECv2-03 (SAST)
with phase-shaped scope notes per D-14: Tool, Make target, CI
placement, Config-file shape, New dev-dep, Exit behavior, Anchor.

Closes SEC-01, SEC-02, SEC-03 — the next security-tooling
milestone has a phase-shaped starting point and does not need to
re-discover.

SECv2-04 (SECURITY.md / disclosure process) stays as a one-liner
per D-13 — distinct concern, separate future-milestone work.

Planning-artifact commit per CLAUDE.md "Changelog Maintenance"
filter (no user-visible surface) — no CHANGELOG entry.
`deprecate()` appends ". See: ..." to each entry, so a trailing
period in the dict value renders as ".. See" in the runtime
DeprecationWarning. The other three entries follow the implicit
no-trailing-period invariant; align 3.0.10-1 with them.

Tests assert ID substring presence, not message text, so this
fix is safe under DEPREC-03.

Surfaces from 05-REVIEW.md CR-01 (was CR-02 in the report; renumbered
during commit-shaping).
Previous recipe `cd docs; pdm run sphinx-build -W ...; cd ..` chained
with semicolons, so the recipe's exit code is the trailing `cd ..`
(always 0). The -W flag emitted "build finished with problems" but
make returned success — the zero-warnings gate Plan 04 installed
was non-functional.

Switched `;` to `&&` and dropped the trailing `cd ..` (Make starts a
fresh shell per recipe line, so the `cd ..` was dead either way).

Empirically verified: deliberately-injected sphinx warning now
returns `make: *** [Makefile:67: docs] Error 1` instead of exit 0.
Clean baseline still passes (no warnings, exit 0).

Surfaces from 05-REVIEW.md CR-01.
Two atomic fixes from 05-REVIEW.md (commits c4a08c3, 73ca4c7)
land as user-facing bug fixes in the active 3.0.15 development
section:

- core.deprecations: trailing period stripped from 3.0.10-1
- dev: make docs && (was ;) so the -W gate actually fails
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR completes Phase 5 of Cement 3.0.16 by documenting and wiring deprecation metadata, enforcement of zero-warning documentation builds, expanding contributor guidelines, and updating project tracking to reflect Phase 5 execution status.

Changes

Phase 5: Deprecations, Docs & Security Stubs

Layer / File(s) Summary
Project State & Phase Tracking
.planning/STATE.md, .planning/REQUIREMENTS.md, .planning/ROADMAP.md, .planning/PROJECT.md
Project status advanced from completed to executing; Phase 4 triage marked complete (manual closure), Phase 5 now active with defined deprecation/docs/security wave plan; triage items TRIAGE-01 through TRIAGE-04 closed, security tooling requirements (pip-audit, bandit, SAST) expanded in v2 spec.
Deprecation Documentation
DEPRECATIONS.md
New in-repo deprecation guide mirrors GitBook narrative; deprecation IDs 3.0.8-1, 3.0.8-2, 3.0.10-1 documented with surface, removal target (v3.2.0), and migration guidance with code examples.
Deprecation Metadata & Docstrings
cement/core/deprecations.py, cement/ext/ext_logging.py, cement/ext/ext_smtp.py
Deprecation removal version pinned to Cement v3.2.0 for IDs 3.0.10-1 (FATAL logging) and 3.0.16-1 (SMTP bool return); docstrings in logging and SMTP handlers corrected to reference v3.2.0 target and proper replacement methods.
Code Corrections & Type Annotations
cement/core/interface.py, cement/utils/shell.py
InterfaceManager.list() return type quoted for autodoc compatibility; InterfaceManager.get() control flow alignment; shell cmd() docstring stderr terminology corrected.
Documentation Build Configuration
Makefile, docs/source/conf.py, README.md
Sphinx build enforced with -W flag (warnings as errors); ReadTheDocs theme config adds version selector option; Travis CI badge removed.
Contributor Guidelines & Changelog
.github/CONTRIBUTING.md, CHANGELOG.md
Guidelines expanded with explicit topic-branch PR policy, git pull --rebase requirement, strengthened make test / make comply (ruff/mypy) checklist, mandatory CHANGELOG.md entries, and Conventional Commits reference; 3.0.15 development section added documenting bug fixes, deprecations, refactoring, and docs updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • datafolklabs/cement#770: Updates deprecation metadata and docs for deprecation ID 3.0.16-1 (SMTPMailHandler.send bool return) that this PR now integrates and finalizes.

Poem

🐰 Hop along through Phase Five we go,
Deprecations documented, clear and slow,
With warnings turned to errors in the build,
Guidelines strengthened, our future gild,
The roadmap marches on with grace,
To v3.2.0's destined place. 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary focus of the pull request—Phase 5 of the Cement 3 modernization, implementing deprecations cleanup, documentation build hygiene, and security-tooling stubs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch modernization-phase-5-pr

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 @.github/CONTRIBUTING.md:
- Line 81: Remove the unused Markdown link reference definition "[Commit
Guidelines]:
http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines"
from the CONTRIBUTING.md content; locate the literal link definition string and
delete that line so the orphan reference is eliminated and the MD053
markdownlint warning is resolved.

In @.planning/ROADMAP.md:
- Line 209: The ROADMAP.md Phase 5 table row "5. Deprecations, Docs & Security
Stubs | 0/6 | Not started | -" is inconsistent with PROJECT.md and STATE.md
which mark Phase 05 complete; update the Phase 5 row in ROADMAP.md to show the
correct progress (e.g., "6/6"), change status to "Completed" (or "Done"), and
replace the "-" with the completion date "2026-05-08" (or equivalent
verification note) so the table aligns with PROJECT.md and STATE.md.

In `@cement/utils/shell.py`:
- Around line 35-36: Docstrings in the shell utilities still misspell stderr as
"stderror"; update the return-type tuple and prose in the function docstring
(the one showing "tuple: When ``capture==True``, returns ``(stdout, stderror,
return_code)``" and the following description) to use "stderr" consistently, and
make the same correction in the exec_cmd docstring (search for "exec_cmd" and
fix any "stderror" occurrences to "stderr"); ensure the example variable names
and prose all match "stderr".

In `@CHANGELOG.md`:
- Line 335: Update the changelog entry on line 335 to accurately describe the
change: replace the incorrect "[docs] Wire api/index into top-level toctree"
with a message reflecting removal of the orphan file (for example "[docs] Remove
orphan api/index.rst; reference api/core/index, api/utils/index, api/ext/index
directly in docs/source/index.rst"). Ensure the new entry mentions deletion of
api/index.rst and that docs/source/index.rst now references the api
subdirectories directly.
🪄 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: b3717264-7e54-442b-aabc-3b84c043e879

📥 Commits

Reviewing files that changed from the base of the PR and between 50970ac and fca6c72.

📒 Files selected for processing (16)
  • .github/CONTRIBUTING.md
  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • CHANGELOG.md
  • DEPRECATIONS.md
  • Makefile
  • README.md
  • cement/core/deprecations.py
  • cement/core/interface.py
  • cement/ext/ext_logging.py
  • cement/ext/ext_smtp.py
  • cement/utils/shell.py
  • docs/source/api/index.rst
  • docs/source/conf.py
💤 Files with no reviewable changes (2)
  • README.md
  • docs/source/api/index.rst

Comment thread .github/CONTRIBUTING.md
[Open Source Initiative]: http://www.opensource.org
[issue tracker]: http://github.com/datafolklabs/cement/issues
[PEP8]: http://www.python.org/dev/peps/pep-0008/
[Commit Guidelines]: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unused link reference definition [Commit Guidelines].

The old [Commit Guidelines] reference was rendered obsolete when the commit guidance was replaced by the Conventional Commits section, but the definition was not removed. This will trigger a markdownlint MD053 warning.

✏️ Proposed fix
 [PEP8]: http://www.python.org/dev/peps/pep-0008/
-[Commit Guidelines]: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines
 [Conventional Commits]: https://www.conventionalcommits.org/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Commit Guidelines]: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines
[PEP8]: http://www.python.org/dev/peps/pep-0008/
[Conventional Commits]: https://www.conventionalcommits.org/
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 81-81: Link and image reference definitions should be needed
Unused link or image reference definition: "commit guidelines"

(MD053, link-image-reference-definitions)

🤖 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/CONTRIBUTING.md at line 81, Remove the unused Markdown link
reference definition "[Commit Guidelines]:
http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines"
from the CONTRIBUTING.md content; locate the literal link definition string and
delete that line so the orphan reference is eliminated and the MD053
markdownlint warning is resolved.

Comment thread .planning/ROADMAP.md
| 4. Backlog Triage | 0/TBD | Not started | - |
| 5. Deprecations, Docs & Security Stubs | 0/TBD | Not started | - |
| 4. Backlog Triage | manual | Complete | 2026-05-05 |
| 5. Deprecations, Docs & Security Stubs | 0/6 | Not started | - |
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Phase 5 progress appears inconsistent with PR completion status.

The Progress table shows Phase 5 as "Not started" with 0/6 plans, but the PR summary states "12/12 must-haves passed" and "Six plans executed across six waves with verification reporting." Additionally, .planning/PROJECT.md and .planning/STATE.md both mark Phase 05 as completed on 2026-05-08.

📊 Proposed fix to align progress table with actual completion
-| 5. Deprecations, Docs & Security Stubs | 0/6 | Not started | - |
+| 5. Deprecations, Docs & Security Stubs | 6/6 | Complete   | 2026-05-08 |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| 5. Deprecations, Docs & Security Stubs | 0/6 | Not started | - |
| 5. Deprecations, Docs & Security Stubs | 6/6 | Complete | 2026-05-08 |
🤖 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 @.planning/ROADMAP.md at line 209, The ROADMAP.md Phase 5 table row "5.
Deprecations, Docs & Security Stubs | 0/6 | Not started | -" is inconsistent
with PROJECT.md and STATE.md which mark Phase 05 complete; update the Phase 5
row in ROADMAP.md to show the correct progress (e.g., "6/6"), change status to
"Completed" (or "Done"), and replace the "-" with the completion date
"2026-05-08" (or equivalent verification note) so the table aligns with
PROJECT.md and STATE.md.

Comment thread cement/utils/shell.py
Comment on lines +35 to +36
tuple: When ``capture==True``, returns ``(stdout, stderror, return_code)``
of the command. ``stdout`` and ``stderror`` are ``bytes`` (the
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

stderror misspelling persists in updated docstring.

Lines 35–36 still read stderror in both the tuple signature and the prose description. The example at line 50 (and the variable name at line 62) correctly uses stderr, so the docstring is inconsistent. The same misspelling exists in exec_cmd's unchanged docstring at line 87.

✏️ Proposed fix
-        tuple: When ``capture==True``, returns ``(stdout, stderror, return_code)``
-            of the command. ``stdout`` and ``stderror`` are ``bytes`` (the
+        tuple: When ``capture==True``, returns ``(stdout, stderr, return_code)``
+            of the command. ``stdout`` and ``stderr`` are ``bytes`` (the

And in exec_cmd (line 87, outside this diff):

-        tuple: The ``(stdout, stderror, return_code)`` of the command.
-            ``stdout`` and ``stderror`` are ``bytes`` (the ``Popen``
+        tuple: The ``(stdout, stderr, return_code)`` of the command.
+            ``stdout`` and ``stderr`` are ``bytes`` (the ``Popen``
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tuple: When ``capture==True``, returns ``(stdout, stderror, return_code)``
of the command. ``stdout`` and ``stderror`` are ``bytes`` (the
tuple: When ``capture==True``, returns ``(stdout, stderr, return_code)``
of the command. ``stdout`` and ``stderr`` are ``bytes`` (the
🤖 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 `@cement/utils/shell.py` around lines 35 - 36, Docstrings in the shell
utilities still misspell stderr as "stderror"; update the return-type tuple and
prose in the function docstring (the one showing "tuple: When ``capture==True``,
returns ``(stdout, stderror, return_code)``" and the following description) to
use "stderr" consistently, and make the same correction in the exec_cmd
docstring (search for "exec_cmd" and fix any "stderror" occurrences to
"stderr"); ensure the example variable names and prose all match "stderr".

Comment thread CHANGELOG.md
- `[dev]` Phase 03 (Internal Refactor & Coverage Hardening) complete:
REFACTOR-01..04 + COV-01..03 satisfied; ROADMAP updated.
- `[docs]` Drop unsupported 'logo' theme option from sphinx conf
- `[docs]` Wire api/index into top-level toctree
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if api/index.rst was deleted or modified
git log --all --full-history --oneline -- '**/api/index.rst' | head -5

# Check for references to api/index in recent commits
git log --all -p --grep='api/index' | head -50

Repository: datafolklabs/cement

Length of output: 232


🏁 Script executed:

# Check if api/index.rst currently exists
find . -name "api" -type d -o -name "index.rst" -type f | grep -E "(api|index)" | head -20

Repository: datafolklabs/cement

Length of output: 204


🏁 Script executed:

# Check for toctree files and their contents
find docs -name "*.rst" -type f | xargs grep -l "toctree" 2>/dev/null | head -5

Repository: datafolklabs/cement

Length of output: 178


🏁 Script executed:

# Look for recent changes involving api/index
git log --all -p --follow -- "docs/api/index.rst" 2>/dev/null | head -100

Repository: datafolklabs/cement

Length of output: 45


🏁 Script executed:

# Check the structure of docs directory
ls -la docs/ 2>/dev/null | grep -E "^d|^-" | head -20

Repository: datafolklabs/cement

Length of output: 222


🏁 Script executed:

# Examine the main index.rst file to see toctree structure
cat docs/source/index.rst | head -80

Repository: datafolklabs/cement

Length of output: 409


🏁 Script executed:

# Look for any toctree directives mentioning api
grep -r "toctree" docs/source/index.rst -A 20 | head -40

Repository: datafolklabs/cement

Length of output: 147


Correct changelog entry to accurately describe the actual change.

The changelog entry "Wire api/index into top-level toctree" is inaccurate. The api/index.rst file does not exist in the repository and was deleted. The toctree in docs/source/index.rst now directly references the api subdirectories (api/core/index, api/utils/index, api/ext/index) rather than routing through a parent api/index.rst file.

Update line 335 to reflect the actual change, such as "[docs] Delete orphan api/index.rst" or similar wording that describes removal rather than wiring.

🤖 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 `@CHANGELOG.md` at line 335, Update the changelog entry on line 335 to
accurately describe the change: replace the incorrect "[docs] Wire api/index
into top-level toctree" with a message reflecting removal of the orphan file
(for example "[docs] Remove orphan api/index.rst; reference api/core/index,
api/utils/index, api/ext/index directly in docs/source/index.rst"). Ensure the
new entry mentions deletion of api/index.rst and that docs/source/index.rst now
references the api subdirectories directly.

@derks derks closed this May 8, 2026
@derks derks deleted the modernization-phase-5-pr branch May 8, 2026 01:34
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