Skip to content

docs: add planning and release model - #147

Closed
bernalde wants to merge 6 commits into
masterfrom
docs/planning-release-model
Closed

docs: add planning and release model#147
bernalde wants to merge 6 commits into
masterfrom
docs/planning-release-model

Conversation

@bernalde

@bernalde bernalde commented Jul 28, 2026

Copy link
Copy Markdown

Action for the team: please finalize this before opening, merging, or
reviewing other unrelated PRs.
The in-flight release PRs #106, #135, and
#131 remain exceptions; #136 has merged.

Summary

Adds PLANNING.md as the version-controlled source for how this repository
plans work and cuts releases. When present on the default branch, it is the
adopted planning, prioritization, and release policy; coding and verification
policy remains in AGENTS.md, adopted through PR #132.

The document covers:

  • A continuous workflow with a contributor-capacity work-in-progress limit.
  • A plain-language release-risk policy for critical and high-severity defects.
  • The first 0.1.0a1 milestone, keyed to durable issue scope with dated PR
    implementation status and issue-based assignment ownership.
  • Project Implement CI/CD pipeline and enhance testing framework for PharmaPy #1 views, fields, automation, and Priority cleanup.
  • A lazy-consensus decision log with a ratifiable release-manager fallback.
  • Event-driven maintenance, including a final pre-merge snapshot refresh.
  • PEP 440 definitions for alpha, release-candidate, and final releases.
  • Open release questions tied to the existing TESTING.md and
    DEPENDENCIES.md sources.

Ratification window

This PR is the ratification record. The review window for revision 511f705
runs from 2026-07-30T14:34:45Z through 2026-08-06T14:34:45Z. The
seven-day duration is one full workweek so asynchronous contributors have each
weekday available. Please record objections on this PR.

Until the window closes, hold new or unrelated pull requests and do not cut the
release tag. PRs #106, #135, and #131 continue because the proposed milestone
depends on them; existing in-flight fixes may land at their owners' discretion.
This transient coordination request intentionally lives here rather than in the
durable policy file.

Review changes

The maintainer review at 2579641 was addressed in cb2c963:

The body-only review at cb2c963 was addressed in 488ded5:

  • Made milestone issues—not PLANNING.md or linked PRs—the authoritative
    assignment record, with the release manager responsible for keeping one
    accountable assignee and Project status current.
  • Defined event-driven maintenance ownership and refresh triggers; individual
    assignment changes remain on issues unless the ownership policy changes.
  • Added a PEP 440 terminology table explaining releases, 0.1.0a1,
    0.1.0rc1, 0.1.0, and numeric suffix progression.

The follow-up review at 488ded5 was addressed in 511f705:

  • Updated the authority text now that PR Add shared coding-agent guidelines #132 and AGENTS.md have landed.
  • Re-ran every dated issue, label, Project, CI, and implementation-status claim
    in one pass at 2026-07-30T14:29:55Z. Project Implement CI/CD pipeline and enhance testing framework for PharmaPy #1 now contains all 86 open
    issues; 58 Priority values remain mirrored and Size remains empty.
  • Added a final pre-merge trigger that refreshes §2 and §5 together after the
    review or ratification window closes.
  • Required clearing the 58 mirrored Priority values; blank now explicitly means
    Needs triage until maintainers set delivery order independently of Severity.
  • Defined blocker, epic, release test suite, and WIP limit; added a WIP example;
    standardized the test-suite name; and replaced the specific jargon identified
    by the reviewer.
  • Fixed the incomplete scope-out sentence and the lone curly-quoted term.

Why a document in a PR, not an issue

The deliverable is a durable artifact from which the milestone, Project views,
and release notes can be generated and against which the process can be
verified. A reviewed file also preserves line-level history. PLANNING.md §9
describes where its durable sections are copied and how the source stays
current.

Verification

  • GitHub REST and GraphQL snapshot at 2026-07-30T14:29:55Z: 86 open issues,
    62 correctness issues, the documented severity distribution, all 86 issues in
    Project Implement CI/CD pipeline and enhance testing framework for PharmaPy #1, 58 mirrored Priority values, no populated Size values, and no
    milestones, releases, or tags.
  • GitHub-rendered Markdown: all 19 headings and 15 authored Markdown links
    resolve to existing anchors, repository files, or the official PEP 440 page.
  • git diff --check passes; PLANNING.md remains UTF-8 with LF endings.
  • Current-head CI at 511f705: Core tests, locked pixi install on Ubuntu,
    locked pixi install on Windows, and Assimulo integration tests all passed.
  • Relative to current master (2cd49c3), the PR changes only PLANNING.md;
    no source, tests, or workflows differ. The merge-tree check is conflict-free.

Branch hygiene

  • Base branch: master; current inspected tip 2cd49c3 includes PR Add shared coding-agent guidelines #132.
  • Head branch: docs/planning-release-model.
  • Current review-fix head: 511f705.
  • Not stacked; no prerequisite PRs.

Review guidance

Reviewers: @andres9403 and @Mazhar331. The remaining decisions to ratify are
collected in PLANNING.md §7 with explicit defaults and ownership. Per §9, the
dated §2 and §5 data must be refreshed together in the final pre-merge revision
after the ratification window closes.

Refs #68, #130, #134, #8, #146

A single version-controlled source for how PharmaPy-org/PharmaPy plans
work and cuts releases: operating model, release-risk policy, the first
milestone (0.1.0a1) with scope mapped to open PRs, Project changes, and
a lazy-consensus decision log. Draft for discussion; adopts nothing until
merged.
@bernalde
bernalde requested review from Mazhar331 and andres9403 July 28, 2026 04:25
@bernalde
bernalde marked this pull request as ready for review July 28, 2026 16:03

@Mazhar331 Mazhar331 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maintainer review of PLANNING.md at 2579641. Thanks for writing this down — the operating model, the severity gate in §4, and the decision log are the right shape for a team this size, and I verified §2 claim by claim rather than taking it on trust. Almost all of it holds exactly (details below).

The one thing that blocks merge is not the content of the policy but what the file says about itself once it is on master.

Blocking

1. The draft banner and the PR-freeze paragraph become false and permanent on merge, and §9 defines no step that removes them.

Lines 3–13 declare Status: DRAFT PROPOSAL — under discussion in this pull request. Not yet adopted. and Nothing here is policy until this PR is merged. Lines 46–52 instruct readers to hold new or unrelated pull requests for a one-week objection window. §9 says "On merge, this document becomes the reference" but lists only how §3–§6 graduate into other artifacts; nothing removes the banner or the freeze.

The result on master is a document that simultaneously declares itself unadopted and is the adopted reference, and that instructs every future reader — including someone arriving in six months — to freeze unrelated PRs against a window whose start date is not recorded in the file. There are ten open PRs right now, six of them correctness fixes (#102, #106, #107, #114, #115, #126); a durable freeze instruction is a real cost to leave lying around.

Either fix works:

  • Add an explicit §9 first step: on merge, replace the banner with Adopted YYYY-MM-DD and delete §1's freeze paragraph. Then this PR is merged only together with that edit, or the edit is the merge commit.
  • Better: keep the freeze request and the objection window out of the file entirely — they belong in the PR body and a tracking issue — and let PLANNING.md carry only the durable policy. §7 can state the ratification mechanism (lazy consensus, one-week window) without pinning a specific in-flight window.

Inline comments carry the rest of the detail.

Nonblocking

  • §5's scope table is already stale: #136 merged at 2026-07-28T18:50Z, after this PR's last update (16:03Z), so this is drift rather than an authoring error — but §5 is written to be pasted verbatim into the milestone description, so it propagates. Details inline (line 145).
  • §2's 85 open issues is now 86, and the snapshot predates #146 (created 2026-07-27T19:06Z) which §5 and §7 D2 both cite. Inline (line 59).
  • §6's "Immediate data fixes" are one-shot tasks in a durable file — once done they stay permanently unchecked in git. Inline (line 220).
  • §8 Q1 and §5's exit criteria re-open questions that TESTING.md on master already answers. Inline (line 253).

Questions

  • D7 (release manager) cannot be ratified by the mechanism §7 defines — silence names nobody, yet §4's promotion gate depends on it. Inline (line 236).
  • How does this relate to PR #132 (AGENTS.md)? Both PRs introduce a canonical policy document and neither references the other. Inline (line 274).

§2 verification (read-only, GitHub + repo, 2026-07-28)

Everything below was re-queried independently. Confirmed exactly as written:

  • No milestones (0), no releases (0), no tags (0); pyproject.toml version = "0.0.1", requires-python = ">=3.9".
  • Open-issue labels: 62 correctness, 60 status:verified, 1 severity:critical, 34 severity:high, 20 severity:medium, 3 severity:low.
  • Severity coverage: exactly 4 open correctness issues carry no severity (#17, #67, #140, #141) → 58 of 62 covered; 0 severity-labelled issues lack correctness. Both halves of the claim hold.
  • #68 is the sole severity:critical.
  • Epics #3, #17, #67, #118 are native issue-type Epic (not just the epic label) — the §3 claim is precise.
  • CI: Core tests pins python-version: "3.11"; Assimulo integration tests is continue-on-error: true; master (bbd7b9a) is green on all four checks.
  • DOI 10.1016/j.compchemeng.2021.107408 resolves via Crossref to the 2021 Computers & Chemical Engineering paper.
  • §4's parenthetical is accurate: PR #106 adds tests/test_crystallizer_energy_balances.py with exactly four regression tests, one per sub-defect.
  • §5's issue↔PR mapping is right: #114#23 (reversible Jacobian), #115#26 (BatchCryst Jacobian sign), #135#134, #131#130, #146 is open and is the distribution-name reservation.
  • pharmapy-sim is not registered on PyPI (404) while PharmaPy is taken by an unrelated project (200) — so §5's and D2's "recheck the name before publishing (#146)" is the correct posture, and the window it warns about is real.
  • All ten TOC anchors resolve to real headings.

Not verified: the Project #1 claims (created 2026-07-21, Priority mirroring Severity, Size 0-of-85, only #23/#26/#134 in an iteration). A ProjectsV2 read needs the read:project token scope, which my token lacks. Worth noting in §2 who can re-run that query, since §6 asks the team to act on it.

Other checks

  • Changed-file set: PLANNING.md only, +279/−0. Local diff from merge-base 4c2d8ea matches gh pr diff --name-only.
  • No whitespace errors (git diff --check); file is UTF-8, LF, no EOL churn (new file).
  • PR head CI (2579641): Core tests success, Assimulo integration tests success. The two Locked pixi install jobs do not appear because the head predates #136; they are green on master.
  • Merge result: git merge-tree --write-tree origin/master 2579641 is conflict-free against the advanced base (bbd7b9a). A docs-only addition, so no build or test impact.
  • master is unprotected (branches/master/protection → 404), so no approving-review count or conversation-resolution gate is enforced.

I would not merge this until the blocking issue above is addressed.

Comment thread PLANNING.md Outdated
formalizes *that*, and deliberately avoids ceremony (sprints, mandatory
estimates, blocking committee gates) that a team this size will not sustain.

**Finalizing this document is the team's current top priority.** While it is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — this paragraph, together with the Status: DRAFT PROPOSAL … Not yet adopted. banner at the top, is written from inside the review window and stays in the file after merge.

Once this lands, master carries a document that says it is not yet adopted while §9 says it is the reference, and that tells every future reader to hold unrelated PRs — with no start date recorded, so the "one week" never expires for someone reading it later. Ten PRs are open right now; six are correctness fixes (#102, #106, #107, #114, #115, #126).

Concretely, I would move the whole freeze request and the specific objection window out of the file and into the PR body plus a tracking issue, leaving §7 to state only the mechanism (lazy consensus, one-week window from the ratification announcement). If you would rather keep it here, then §9 needs a first step that is unambiguous, for example:

On merge (before anything else): replace the DRAFT banner with Adopted YYYY-MM-DD, delete §1's PR-hold paragraph, and open the ratification issue that starts the objection window.

Either way the file that ends up on master should be true about its own status.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963. I removed both the draft banner and the complete
PR-hold paragraph from PLANNING.md. The durable status now says the document
is repository policy when present on the default branch. The transient
ratification request and exact 2026-07-29T13:59:19Z
2026-08-05T13:59:19Z window live only in the PR body; §7 retains the general
lazy-consensus mechanism and explains why the window spans one full workweek.

The current-head GitHub render and all four CI checks pass.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: the durable default-branch status and
removal of the permanent PR freeze remain unchanged. The new assignment,
maintenance, and version-term clarifications do not reintroduce transient
review state into PLANNING.md; all four CI checks pass.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 511f705: the durable adopted-on-merge status
remains intact, and all transient hold/window text remains confined to the PR
body. The follow-up only refreshes data and improves policy wording.

Comment thread PLANNING.md Outdated
Queried read-only from GitHub on 2026-07-27. **Time-sensitive** — re-verify
before acting on any count.

- 85 open issues; **no milestones, no releases, no Git tags**; `pyproject.toml`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nonblocking — two small drifts in the snapshot.

85 open issues is 86 as of today. More interestingly, the count predates #146, which was created 2026-07-27T19:06Z and which this document cites in both §5 and §7 D2 — so §2 and the body were captured at different points on the same date.

Everything else in §2 I re-queried and it holds exactly, including the parts that are easy to get subtly wrong: 4 correctness issues without severity (#17, #67, #140, #141) and zero severity-labelled issues that are not correctness.

Suggest a timestamp rather than a bare date, since the section is explicitly time-sensitive:

Suggested change
- 85 open issues; **no milestones, no releases, no Git tags**; `pyproject.toml`
Queried read-only from GitHub on 2026-07-27T19:30Z. **Time-sensitive** — re-verify
before acting on any count.

(Adjust to the real capture time.) Separately: I could not verify the Project #1 bullet below — a ProjectsV2 read requires the read:project token scope. Since §6 asks the team to act on those numbers, it is worth saying in §2 who can re-run that query.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963. I re-ran the issue, label, milestone, release, tag, and
Project queries at 2026-07-29T13:54:25Z; the snapshot now reports 86 open
issues and retains the independently confirmed severity counts. The section
names both REST and GraphQL as its sources and states that Project #1
re-verification requires organization access plus read:project. The
ProjectsV2 query succeeded in this pass and confirmed 85 items, no populated
Size values, and the three iteration assignments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: §2 remains an explicitly timestamped
REST/GraphQL snapshot with the read:project requirement. The new maintenance
contract additionally forbids advancing that timestamp unless every snapshot
claim is re-queried in the same pass.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Refreshed again at 511f705 in one REST/GraphQL pass. §2 now records
2026-07-30T14:29:55Z, all 86 open issues are in Project #1, and the documented
label, Size, Priority, and iteration counts match that same pass.

Comment thread PLANNING.md Outdated
| #68 | PR **#106** | The critical crystallizer crash + enthalpy basis (hard gate). |
| #134 | PR **#135** | Solver-free model imports (lazy Assimulo). |
| #130 | PR **#131** | Reproducible, public documentation (tracked as the outcome; #131 is one step and intentionally does not close #130). |
| packaging / clean install | PR **#136** | `pharmapy-sim` distribution, pixi environments, install guide, two-platform locked install matrix. Recheck the distribution name before publishing (#146). |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nonblocking#136 merged at 2026-07-28T18:50Z, after this PR was last updated (16:03Z), so this is ordinary drift and not an authoring error. It still matters because §5 is explicitly written to be pasted verbatim into the milestone description, so the stale row propagates into GitHub.

Two knock-on effects at current master (bbd7b9a):

  • The exit criterion "A clean environment installs the package (pip and pixi), verified by a smoke-install/import job" is already satisfied — Locked pixi install (ubuntu-latest) and (windows-latest) are gating checks, and INSTALLATION.md now exists.
  • §1's request that Add pharmapy-sim distribution, pixi environments, and install guide #136 "continue to merge" is moot.

The general point: §2 flags itself as time-sensitive, but §5's Delivered by column hard-codes the same kind of mutable state without that caveat. PRs merge; issues are stable. Consider keying the table on issues and treating the PR column as a dated snapshot, with a one-line note to that effect above the table.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963 after merging current master (bbd7b9a). The scope
table is now keyed to durable issues, labels PR implementation state as a dated
snapshot, and records #136 as merged. The clean-install exit criterion is
checked with references to the Linux/Windows locked-install jobs,
INSTALLATION.md, and TESTING.md; #136 is also gone from the PR body's
in-flight exception list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: milestone scope remains keyed to issues,
PR implementation state remains a dated snapshot, and #136 remains recorded as
merged with the clean-install criterion satisfied.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 511f705: scope remains keyed to durable issues,
§5 carries the refreshed implementation timestamp, #136 remains recorded as
merged, and the clean-install criterion remains satisfied.

Comment thread PLANNING.md Outdated

**Immediate data fixes** (independent of the decisions):

- [ ] Put #68 into active focus — it is the only hard release blocker.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nonblocking — these two are one-shot operational tasks, not policy. Once someone does them, the boxes stay unchecked in git forever and read as outstanding work to the next person who opens the file.

Same concern, milder, applies to §5's entry/exit checkboxes — though those are defensible since the section is meant to be copied into a milestone description where they will get checked.

Suggest moving both to the tracking issue (or straight onto #68 and #134) and leaving §6 with just the durable Views/Fields/Automations policy.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963. I removed both one-shot checkboxes from §6. The durable
policy now says operational state is maintained on its owning issue: #68 owns
hard-blocker status, and #134 owns its assignee and Project status. The
milestone entry/exit checklist remains because §5 explicitly generates the
milestone description where those boxes are updated.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: §6 still contains no one-shot
operational checkboxes. The new assignment clarification reinforces that live
ownership and handoffs are maintained on milestone issues and in Project
status, not duplicated as durable-file tasks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 511f705: §6 still has no stale operational
checkboxes. Live issue ownership remains on #68/#134; the Project field rules
now also specify how mirrored Priority data is cleaned up.

Comment thread PLANNING.md Outdated
| **D4** | Priority model | Re-triage independently of severity (or collapse to `Now/Next/Later`) | Yes | maintainer |
| **D5** | `Size` field | Not required; optional for near-term items only | Yes | team |
| **D6** | #23/#26 in this release? | No unless the owner confirms they ship here | Yes | issue owner |
| **D7** | Release manager | Name one person; they decide high→blocker promotions | Yes | org |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question — D7 is the one row that lazy consensus cannot resolve. Every other decision has a concrete proposed default that silence ratifies; this one's default is a restatement of the decision ("Name one person"), so a week of silence produces no release manager.

That matters because §4's promotion rule — the core of the release-risk policy — has no actor without it: "A named release manager decides promotions."

Could you either name a person in the default, or give a fallback rule that silence can ratify, e.g. "the milestone owner acts as release manager unless someone else volunteers"? Also worth reconciling the Owner column: D7's owner is org, but the decision it produces is what makes §4 executable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963. D7 now has a concrete default that silence can ratify:
the maintainer who creates the milestone acts as release manager unless the
milestone names another volunteer. Its owner is now maintainer, so the
default and §4's promotion authority are executable without a separate naming
decision.

@bernalde bernalde Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: D7 retains the ratifiable milestone
creator fallback and maintainer owner. The assignment clarification now uses
that named release manager as the person responsible for unowned milestone
work and current issue/Project ownership.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 511f705: D7 still names the milestone creator as
the default release manager unless the milestone names another volunteer, so
silence yields an executable owner.

Comment thread PLANNING.md Outdated

These need a human decision; do not invent answers.

1. **Smoke/validation suite.** What is the minimal numerical/smoke suite whose

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

NonblockingTESTING.md on master already answers a good part of Q1, and this section reads as if it were an open design question.

It documents the concrete lanes (python -m pytest tests/ -m "not assimulo", pixi run test, pixi run -e assimulo test-assimulo), states that GitHub Actions gates the locked pixi install on Linux and Windows, and states that the Assimulo job is "intentionally informational" — which is exactly the blocking-versus-non-blocking distinction §4 and §5 need.

Reframing Q1 as "which of TESTING.md's existing lanes gate the release, and what numerical assertions do we add on top?" makes it answerable, and it avoids §5's exit criteria silently forking from the file that actually documents the test lanes. DEPENDENCIES.md is likewise the natural home for the "dependency policy is explicit" exit criterion. Neither file is referenced anywhere in this document.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963 against the post-#136 base. Q1 now starts from
TESTING.md's existing core pytest, locked pixi, and informational Assimulo
lanes and asks only which lanes gate the release and which numerical assertions
must be added. The exit criteria link DEPENDENCIES.md as the source for bounds
and environment policy and link INSTALLATION.md/TESTING.md for the completed
clean-install criterion.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: TESTING.md, DEPENDENCIES.md, and
INSTALLATION.md remain the authoritative detailed sources. The new text is
limited to assignment maintenance and PEP 440 terminology.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 511f705: TESTING.md, DEPENDENCIES.md, and
INSTALLATION.md remain the detailed sources. Q1 now consistently calls the
still-to-be-defined gate the “release test suite.”

Comment thread PLANNING.md Outdated

- §4 (release-risk policy) and §5 due-date/hygiene → the durable content of a
future `RELEASING.md`.
- §3 (operating model) → the basis of `CONTRIBUTING.md` / the Project README.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question — how does this document relate to PR #132, which adds AGENTS.md as "the canonical repository vision and coding policy" and states that it documents "issue/PR coordination"?

Two policy documents are in flight, each presenting itself as canonical, and neither references the other. §9 here proposes graduating §3 into a CONTRIBUTING.md that does not exist yet, while #132 is landing a policy file that arguably already occupies part of that space.

A clean split would be: AGENTS.md = how code is written and verified; PLANNING.md = how work is planned, prioritized, and released; CONTRIBUTING.md (later) = the contributor-facing summary pointing at both. Whichever of #132 / #147 merges second should add the cross-reference. Worth settling before either lands, so the repo does not end up with two documents claiming the same authority.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in cb2c963. The opening status block and §9 now define the split
explicitly: PLANNING.md owns planning, prioritization, and releases; PR #132's
proposed AGENTS.md owns coding and verification rules; a future
contributor-facing CONTRIBUTING.md summarizes and points to both instead of
duplicating either.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Current-head confirmation at 488ded5: the PLANNING.md/proposed AGENTS.md
authority split remains explicit. The new maintenance section stays within
PLANNING.md's planning, prioritization, and release scope.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated at 511f705 for the live state: PR #132 has merged, so both the opening
status block and §9 now state that AGENTS.md owns coding and verification
rules while PLANNING.md owns planning, prioritization, and releases.

@bernalde bernalde changed the title docs: planning and release-model proposal (draft, for discussion) docs: add planning and release model Jul 29, 2026
@bernalde

Copy link
Copy Markdown
Author

Addressed the maintainer review on current head cb2c963.

Changes

  • Made PLANNING.md durable on merge: removed the draft banner and permanent
    PR freeze, and kept the dated ratification window only in the PR body.
  • Re-queried the mutable snapshot through GitHub REST/GraphQL at
    2026-07-29T13:54:25Z, documented the read:project requirement, and
    recorded PR Add pharmapy-sim distribution, pixi environments, and install guide #136 as merged.
  • Keyed milestone scope to issues, removed one-shot Project checkboxes, gave D7
    a ratifiable release-manager fallback, and linked the existing testing,
    dependency, and installation sources.
  • Defined the authority split with PR Add shared coding-agent guidelines #132: PLANNING.md owns planning,
    prioritization, and releases; the proposed AGENTS.md owns coding and
    verification; a future CONTRIBUTING.md points to both.
  • Audited numeric policy choices in the document. Mutable counts are sourced
    and timestamped; WIP and objection-window choices now have a formula or
    explicit rationale instead of unexplained literals.

Verification

  • GitHub-rendered Markdown: 17 headings and 14 links validated.
  • git diff --check: pass; PLANNING.md remains UTF-8/LF.
  • Current-head CI: Core tests, locked pixi install on Ubuntu, locked pixi
    install on Windows, and Assimulo integration tests all passed.
  • GitHub reports the PR clean and mergeable at cb2c963.

All seven reviewer threads have current-head replies below and remain unresolved
for the reviewer to verify. The formal review decision remains
CHANGES_REQUESTED pending re-review.

@andres9403 andres9403 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In general, I really like that this document makes the refactoring and release plan explicit. It gives the team a clearer source of truth instead of relying on scattered GitHub state.

I have a few comments/questions:

For me it is not fully clear if the PR or issue assignments for the milestone will live. Will assignments be listed in PLANNING.md, It may help to state who is responsible for assigning work and keeping that assignment current.

Since PLANNING.md is described as a living planning record, I think it would help to define how it is maintained: who updates it, when it should be refreshed, and whether updates happen before each milestone/release, on a fixed cadence, or only when planning decisions change.

The versioning proposal is useful, but some of the nomenclature may not be obvious. Terms like 0.1.0a1 or rc1, releases, maybe need an explanation.

@bernalde

Copy link
Copy Markdown
Author

Addressed the new body-only review from andres9403 on current head
488ded5.

Changes

  • Made milestone issues the authoritative assignment record. The release
    manager assigns unowned milestone work and ensures one accountable issue
    assignee plus Project status remain current; PR roles do not replace issue
    ownership.
  • Added an event-driven maintenance contract for PLANNING.md: the release
    manager owns milestone synchronization and refreshes the dated snapshot and
    implementation status before milestone creation/rollover, before any tag,
    and when a ratified decision changes scope, the operating model, or a release
    gate. Individual assignment handoffs remain on issues.
  • Added an official PEP 440 terminology table explaining releases, the first
    alpha (0.1.0a1), first release candidate (0.1.0rc1), final release
    (0.1.0), and how phase suffixes advance.
  • Refreshed the PR body to the new head and its exact one-workweek ratification
    window.

Verification

  • GitHub-rendered Markdown: 19 headings and 15 authored links validated,
    including the official PEP 440 source.
  • git diff --check: pass; PLANNING.md remains UTF-8/LF.
  • Current-head CI: Core tests, locked pixi install on Ubuntu, locked pixi
    install on Windows, and Assimulo integration tests all passed.
  • GitHub reports the PR clean and mergeable at 488ded5.

The new review contained no inline targets, so its three body-only findings are
covered above. All seven earlier reviewer threads have fresh current-head
replies and remain unresolved for reviewer verification. The formal review
decision remains CHANGES_REQUESTED pending re-review.

@bernalde
bernalde requested a review from Mazhar331 July 29, 2026 21:55
@Mazhar331

Copy link
Copy Markdown
Collaborator

Read-only verification pass over head 488ded5 against my review at 2579641 and @andres9403's body-only review. All ten items are addressed at head, verified against the file rather than the summaries. Three new items below, plus a readability concern I think is worth taking seriously.

Round 1 (my review) — all seven addressed

Finding Status Verified at head
Blocking draft banner + permanent PR freeze Addressed Banner is now "Repository policy when present on the default branch" (L3); §1's freeze paragraph is gone; §7 states the mechanism only and explicitly says the announcement belongs in a PR or tracking issue (L251–256). The dated window and the hold request now live in the PR body — exactly the split I asked for.
§2 snapshot drift Addressed Now timestamped 2026-07-29T13:54:25Z with the read:project requirement stated. I re-queried every count today: 86 open issues, 62/60/1/34/20/3 labels, 0 milestones/releases/tags — all still exact.
§5 stale #136 row Addressed Table rekeyed to issues with an explicit dated-snapshot caveat (L153–155); #136 recorded merged; clean-install exit criterion now [x] citing the Linux/Windows jobs, INSTALLATION.md, TESTING.md.
§6 one-shot checkboxes Addressed Both removed; replaced by the durable rule that operational state lives on #68 and #134 (L245–247).
Question D7 not ratifiable Addressed Default is now "the maintainer who creates the milestone acts as release manager unless the milestone names another volunteer", owner maintainer. Silence now resolves it, so §4's promotion authority is executable.
TESTING.md / DEPENDENCIES.md unreferenced Addressed §8 Q1 starts from TESTING.md's existing lanes; exit criteria link all three files. All three resolve on master.
Question overlap with #132 Addressed (see new item 1) Split is stated in the header block and §9 — though the wording has since gone stale.

Round 2 (@andres9403) — all three addressed

  • Where milestone assignments live / who keeps them current → §5 L214–220: assignments live on milestone issues, one accountable assignee each, release manager responsible for unowned work and handoffs.
  • How the document is maintained and when it refreshes → new §9 "Maintenance responsibility and refresh triggers" (L317–334), event-driven with three explicit triggers and a rule against advancing the snapshot timestamp without re-querying.
  • Version nomenclature (0.1.0a1, rc1, "release") → new §5 "Version terminology" table with a PEP 440 link.

New since 488ded5

1. AGENTS.md is no longer "proposed" — PR #132 merged at 2026-07-29T21:53:44Z, about five hours after this head was pushed. AGENTS.md, CLAUDE.md, and .github/copilot-instructions.md are on master at 2cd49c3. Two lines are now factually wrong:

  • L11–12: "Coding and verification rules belong in AGENTS.md if that file is adopted" — it is adopted.
  • L313: "the proposed AGENTS.md owns coding and verification rules".

The PR body's Summary carries the same "proposed in AGENTS.md by PR #132" phrasing. Dropping the conditional in all three places is the whole fix. This is the same drift pattern as the #136 row last round, and the new §9 maintenance rule is what should catch it — worth confirming the rule works on its first real test.

Related: my round-1 comment said whichever of #132/#147 landed second should add the cross-reference. #132 landed first and AGENTS.md does not mention PLANNING.md — its opening line claims to be "the canonical project guidance for coding agents" with no boundary statement. So the cross-reference is entirely this PR's to carry, which it does. A one-line pointer back from AGENTS.md would be worth a small follow-up PR, but it is out of scope here.

2. §2 has an internal arithmetic gap: 86 open issues (L54) but Project #1 "contains 85 items" (L63–64), both from the same 13:54:25Z pass. §6 keeps "auto-add of open issues", so under that automation the two numbers should match. Either one open issue is genuinely missing from the Project — which is a small data fix worth naming, and precisely the kind of thing §2 exists to surface — or the two figures came from different moments despite the shared timestamp. I could not check this myself: a ProjectsV2 read needs the read:project scope, which my token lacks. Could you confirm which it is?

3. Two small nits. L169 reads "Every correctness issue counted in §2 as a single release gate" — the rewrite that removed the hard-coded 62 dropped the sentence's verb; "Treating every correctness issue counted in §2 as a single release gate" restores it. And L139 has the file's only curly quotes (“Final”) where every other quotation mark is straight ASCII.

Readability — I agree this is a real concern

Raising this as a reviewer because @andres9403 already raised a slice of it ("some of the nomenclature may not be obvious") and the fix addressed only the version strings. The broader issue stands, and for a planning document it matters more than it would in code: this file's stated purpose is that the team agrees on an operating model and walks the exit checklist. A gate the team cannot read is not a gate.

The strongest single instance is concrete and fixable:

  • "smoke suite" is a release gate and is defined nowhere. It appears five times (L113, L117, L166, L189, L196), in §4's promotion rule, §5's exit criteria, and §8's Q1 — and in three different spellings: "smoke/validation suite", "smoke/numerical suite", "smoke-install jobs". It appears in no other document in the repository. §4 says a severity:high defect is promoted to a release blocker if it "affects a module exercised by the release smoke/validation suite", so whether a defect blocks the release currently depends on an undefined term. Pick one name, define it in one sentence, and use it consistently.

Others worth a plain-language pass:

  • L115 "The gate is behavioral, not lexical" — the point (don't trust label presence, check that the tests actually pass) is good; the phrasing needs a dictionary.
  • L91 "Not used as a commitment unit", L276 "as the backlog burns down", §9's "graduate", D1's "Hard once tagged/DOI'd" — project-management idiom that a chemical-engineering contributor has no particular reason to know.
  • L83's WIP formula ("one active item per active contributor plus one shared review/unblocking slot") is more precise than the old "≤ 6" but harder to read; a worked example would fix that.

To be clear about what is already good: "lazy consensus" is parenthetically defined on first use, "work-in-progress (WIP)" is expanded, and the new PEP 440 table is exactly the right pattern. The suggestion is to apply that same pattern to the remaining terms — a short glossary, or inline definitions on first use, covering at minimum smoke suite, blocker/promotion, and epic. That is a docs edit, not a rethink of the model, which I think is sound.

Evidence

  • No local test run: docs-only PR, one file, PLANNING.md alone.
  • CI at 488ded5: Core tests, Locked pixi install (ubuntu-latest), Locked pixi install (windows-latest), Assimulo integration tests — all success.
  • Re-queried every §2 claim independently today; all still exact.
  • All 10 internal anchors resolve against the file's 19 headings; TESTING.md, DEPENDENCIES.md, INSTALLATION.md all exist on master.
  • git merge-tree against current master (2cd49c3, which now includes Add shared coding-agent guidelines #132) is conflict-free.

Status

Every review comment is content-addressed and @andres9403 has approved at this head. reviewDecision still reads CHANGES_REQUESTED because my round-1 review holds that gate — that clears only when I submit a new review, not through replies or another approval. I'll re-review once the AGENTS.md-is-merged wording is corrected; items 2 and 3 and the readability pass are non-blocking and can land in the same push or a follow-up, your call.

@Mazhar331 Mazhar331 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issues with the current snapshot and re-triaging (see inline). Overall, a helpful document but difficult to read and understand. AI agents and those familiar with this line of work might be able to understand, but those of us who are new to package development and the associated terminology and policies will find it difficult. This is especially critical when this doc plans work and all contributors are expected to follow it. I would suggest keeping the core meaning but writing in simpler 'human-like' language.

Comment thread PLANNING.md Outdated
*that*, and deliberately avoids ceremony (sprints, mandatory estimates,
blocking committee gates) that the team will not sustain.

## 2. Verified snapshot (2026-07-29)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Regenerate the snapshot before merging PLANNING.md as other PRs can move meanwhile, and the snapshot predates the merge

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed at 511f705: I refreshed every §2 and §5 snapshot claim together at
2026-07-30T14:29:55Z and added a required final refresh after the ratification
window and before merge.

Comment thread PLANNING.md
release manager owns milestone-specific maintenance: keeping the milestone
description synchronized with §5 and confirming the release gates before a tag.

Maintenance is event-driven rather than scheduled on a fixed calendar. Refresh

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The refresh triggers cover this document's life after merge but not the merge itself. §2 is stamped 2026-07-29T13:54:25Z and the PR body's ratification window runs to 2026-08-05T17:00:58Z, so this file will land at least a week after its own snapshot — with #106, #135, and #131 all able to move in between. Could you add a fourth trigger, and make it cover both timestamped blocks (§2's counts and §5's implementation-status table)?

  • before merging a revision of this document whose snapshot predates the merge;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed at 511f705. §9 now requires a final revision after the review or
ratification window and before merge, and explicitly says the dated §2 snapshot
and §5 implementation status are refreshed together.

Comment thread PLANNING.md Outdated
| **Status** | Workflow state only: `Todo` → `In progress` → `Done`. Delivery is controlled by a **work-in-progress (WIP) limit** on `In progress`, not by sprints. Set the cap when a milestone opens using **one active item per active contributor plus one shared review/unblocking slot**; the formula keeps the limit tied to actual team capacity instead of a magic number. |
| **Milestone** | A repository release (or a concrete, externally meaningful outcome). **Exactly one open at a time** because the Project roadmap already holds longer-horizon work; a second open milestone would create a competing release gate. |
| **Epic + sub-issues** | Decomposition of a large initiative, using native issue-type `Epic` and parent/sub-issue links. Current epics: #3, #17, #67, #118. This is the roadmap's backbone. |
| **Priority** | Maintainer delivery order — **re-triaged independently of severity** (see §7 D4). Until re-triaged it is unreliable and is kept out of delivery views. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"Until re-triaged it is unreliable" has no way to end. §6 stops the Severity→Priority mirroring but leaves the ~58 already-mirrored values in the field, so anyone opening Project #1 sees a populated, authoritative-looking Priority column while the caveat that it's meaningless lives only in this file. Could §6 instead clear the mirrored values? An empty field is self-documenting and removes the misleading signal rather than documenting around it. A Needs triage option would also work.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed at 511f705. §6 now says to clear all 58 Priority values copied from
Severity; blank means Needs triage, and maintainers repopulate Priority only
from delivery order.

@bernalde

Copy link
Copy Markdown
Author

Addressed the follow-up review on current head 511f705.

Changes

  • Refreshed every dated issue, label, Project, CI, and implementation-status
    claim together at 2026-07-30T14:29:55Z. Project Implement CI/CD pipeline and enhance testing framework for PharmaPy #1 now contains all 86 open
    issues; 58 Priority values are mirrored from Severity and Size is empty.
  • Added a final pre-merge refresh trigger for both §2 and §5 after the review or
    ratification window closes.
  • Required the 58 mirrored Priority values to be cleared; blank now means
    Needs triage until maintainers set delivery order independently.
  • Updated the AGENTS.md wording after PR Add shared coding-agent guidelines #132 merged and fixed both small text
    nits.
  • Added plain-language definitions for blocker, epic, release test suite, and
    WIP limit; included a WIP example and replaced the specific jargon identified
    in review.

Verification

  • GitHub-rendered Markdown: all 19 headings and 15 authored links resolve.
  • git diff --check: pass; PLANNING.md remains UTF-8 with LF endings.
  • Merge-tree against current master (2cd49c3): conflict-free; the PR changes
    only PLANNING.md.
  • Current-head CI: Core tests, both locked pixi install jobs, and Assimulo
    integration tests all passed.

No review item was intentionally left unaddressed. The live Project values have
not been mutated by this documentation PR; §6 now defines their required
cleanup. The ratification window now runs through 2026-08-06T14:34:45Z, and
the dated snapshot remains a required final pre-merge refresh after that
window. GitHub still records CHANGES_REQUESTED until the reviewer updates or
dismisses the earlier review.

@bernalde

Copy link
Copy Markdown
Author

Closing this proposal because we no longer accept the premise of a committed
“living” planning document.

PLANNING.md combines durable policy with mutable issue counts, PR status,
Project configuration, release scope, assignments, and temporary decisions.
That makes the repository copy predictably stale and creates a second,
maintenance-heavy source of truth.

Going forward:

  • live scope, assignments, priority, and status belong in issues, milestones,
    and Project Implement CI/CD pipeline and enhance testing framework for PharmaPy #1;
  • temporary release decisions and coordination belong in their owning GitHub
    discussion; and
  • any genuinely durable release or contributor policy should be proposed
    separately in a small, focused RELEASING.md or CONTRIBUTING.md, without
    snapshots of live GitHub state.

This decision supersedes the ratification window and review request recorded in
this PR. The branch and commit history are retained for reference; no reviewer
action is needed on the unresolved threads.

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.

3 participants