Restructure as an R + Python monorepo - #146
Draft
jat255 wants to merge 25 commits into
Draft
Conversation
Binds this workspace to the commons-python kata project and adds the agent operating contract to AGENTS.md (CLAUDE.md symlinks to it). The issue ledger itself lives in KATA_HOME rather than the repo, and is seeded from docs/superpowers/plans/2026-08-18-commons-python-port.md: eight milestone epics wired by blocked-by, the eight Milestone 1 tasks as children of the M1 epic, and the Python-side follow-ups from the 2026-08-19 planning meeting.
The Python work now lands in this repo under pkg-py/ rather than a separate posit-dev/commons-python, so the kata project covers both languages and Python issues carry a py label.
Anyone working in this repository reads AGENTS.md, and kata adoption is per-developer: the ledger is machine-local, so a teammate without kata installed gains nothing from the contract. Lead with a conditional so those agents skip the section instead of being told to use a tool they do not have. Also replaces the generated Graphviz diagram with the same guidance as prose, cutting the section from 56 lines to 21. The conditional sits above the managed markers so re-running kata init --with-agents cannot drop it.
First step of the R + Python monorepo restructure (kata z26z, plan D7). This commit is deliberately rename-only so the relocation can be verified mechanically with `git show --stat -M`; no file contents change. The repository is in a knowingly broken state here: the workflows still point at root-relative paths and are fixed in the commits that follow, along with the install instructions that now need subdir = "pkg-r". LICENSE and LICENSE.md move with the package because DESCRIPTION declares `License: MIT + file LICENSE` and R CMD check resolves it inside the package. Root copies are restored later so GitHub still detects the license for the repository as a whole.
A gitignore pattern containing an internal separator is anchored to the
directory of the .gitignore that declares it, so moving the package under
pkg-r/ silently stopped five patterns from matching anything:
inst/resources/ inst/tiles/ inst/pkgs inst/hex/output src/*.{o,so,dll}
That is a quiet failure rather than a loud one: compiled objects and
generated asset directories simply become committable, and nothing
complains until something large lands in a commit.
Split the file instead of prefixing the paths. Patterns relative to the
package now live in pkg-r/.gitignore, and the root keeps only separator-free
patterns, which match at any depth and therefore cover pkg-py/ as well.
`docs` is one of those: pkgdown writes to pkg-r/docs/, and the root pattern
still covers it.
Verified with `git check-ignore -v` against each pattern.
Adds `paths:` filters and points each workflow at the relocated package. All four filters include tests/shared/** as well as pkg-r/**, because the shared fixtures are the authority for behavior both language packages must agree on and a change to one has to re-run the other's suite. Each filter also lists its own workflow file so edits to CI still trigger CI. `defaults.run.working-directory` applies only to `run:` steps, so the r-lib actions take their own `working-directory` input instead; both setup-r-dependencies and check-r-package accept one. R-CMD-check has no `run:` steps at all and therefore needs no job default, while pkgdown and citation-browser have one and two respectively. Two workflows the plan did not account for: - deploy.yml pushes `path: inst` to Connect, now pkg-r/inst. Left alone it would have kept deploying successfully from a directory that no longer exists. - citation-browser.yaml runs devtools::test() and needs the same treatment as R CMD check. pkgdown's deploy step keeps `folder: pkg-r/docs` spelled from the repository root: the JamesIves action resolves it there, not from the job's working directory.
States what the directory is for and what belongs in it, which is the keystone of the monorepo decision (plan D7): the layout only pays for itself if both suites actually read these fixtures instead of growing parallel copies. No fixtures yet. They get extracted as the provenance and citation code is ported, while the R original and the Python port are both in front of the person doing it, together with the R-side runner that makes the authority claim real.
py-check runs uv sync / ruff / pytest across ubuntu and macos on Python 3.10 and 3.13, scoped to pkg-py/** and tests/shared/**. The plan asks for this to be green against an *empty* pkg-py/, which it cannot literally be: uv sync needs a pyproject.toml, and pytest exits 5 on a suite with no tests, which CI reads as failure. A workflow that passes by doing nothing would be worse than no workflow, so pkg-py/ gets the smallest skeleton that makes the checks mean something — a placeholder pyproject with dev dependencies, and one placeholder test. The scaffold task still owns the real package definition (hatchling, the src/commons/ layout, the extras, runtime dependencies) and deletes both placeholders when it lands. `[tool.uv] package = false` keeps uv from building a distribution with no source tree. Verified by running all three CI commands locally: sync resolves, ruff passes, pytest collects and passes one test.
This is the user-visible break from the restructure, so it gets its own commit. Spelled as pak's path form, `posit-dev/commons/pkg-r`, rather than the plan's `subdir = "pkg-r"`: the latter is remotes::install_github()'s argument, and these instructions use pak, whose GitHub source syntax takes the subdirectory in the path. Both files change together because README.md is generated from README.Rmd. There were exactly two occurrences in the repository, not the wider sweep across vignettes and inst/skills/ the plan anticipated.
The root README describes the layout and why the two packages share a repository; pkg-r/README.md remains the R package's own README. LICENSE and LICENSE.md are restored at the root as copies. They have to exist inside pkg-r/ because DESCRIPTION declares `License: MIT + file LICENSE` and R CMD check resolves it relative to the package, and GitHub only detects a repository's license from the root. Duplication is the cost of satisfying both. CODEOWNERS routes review per language, with tests/shared/ and .github/ going to both sides since a change there alters the contract for both implementations. Handles resolved from the commit history via the GitHub API rather than guessed, except @jat255, whose commits are not yet on the remote. AGENTS.md previously pointed at README.Rmd at the root and read as though one package existed. It now names both packages, says to work from the package directory rather than the root, and states the tests/shared/ rule that makes the monorepo worth having. CLAUDE.md remains a symlink to it.
Written for someone whose muscle memory puts R tooling at the repository root. Leads with the single change that everything else follows from (commands run from pkg-r/), then covers in-flight branch rebasing, why .gitignore split, the CI path filters, and the two new conventions. Calls out the one item that needs repository-admin action rather than a code change: `paths:` filters leave a required status check that never runs pending forever, which blocks merges under branch protection.
- deploy.yml had no `paths:` filter, so a Python-only pull request would have triggered a Connect deploy of the R app. cleanup-previews stays unfiltered on purpose, so it can never leave a stale preview behind. - py-check ran `uv sync`, which rewrites a stale uv.lock in place and lets CI pass on dependency metadata that cannot be reproduced from the commit. `--locked` fails instead. Verified against the committed lockfile. - MIGRATING.md told an in-flight branch to `git rebase main`. `git pull` on a feature branch does not move local `main`, so that can replay the work onto the pre-restructure tree and turn a clean rebase into resolving the entire move by hand. Now `git fetch origin && git rebase origin/main`. - MIGRATING.md claimed every workflow runs with `working-directory: pkg-r`, which was broader than the truth: deploy and cleanup-previews have no working directory and py-check uses pkg-py. Enumerated instead. Also ignores *.Rcheck/ and *.tar.gz under pkg-r/. Neither was ignored before the split, so running R CMD check in the package directory left build output staged for commit.
The previous fix enumerated the workflows correctly but kept the "every workflow" opener, so the paragraph asserted a rule and exempted a workflow from it two sentences later. Also notes, once rather than per workflow, that each filter includes its own file so editing CI still triggers CI.
- Said the R package is "released and in use", which contradicts AGENTS.md in the same commit: neither package has been publicly released. - Described tests/shared/ in the present tense as fixtures CI enforces and both suites read. It holds only the contract right now; the fixtures land as the provenance and citation code is ported. Claiming enforcement that does not exist is the same failure the directory is meant to prevent.
"Complete" overcorrected the previous "released and in use". pkg-r/README carries a lifecycle-experimental badge and warns that the interface should be expected to change, so the root README now says the same thing.
Both READMEs said each test suite reads tests/shared/ directly. That cannot work for the R side: testthat needs its fixtures inside the package, and an installed R package cannot reach outside its own directory. The R suite therefore consumes a copy synced into pkg-r/tests/testthat/fixtures/, committed, with CI failing when the copy is stale. shinychat does exactly this with its own shared matrix fixture, which is where the correction comes from. Distinguishes the two cases explicitly, since the original wording made a synced copy sound like the thing it forbids: a generated copy is the mechanism, a hand-edited one is the defect. Recorded as D10 in the port plan; the same arrangement covers the system prompt and browser assets when those move to shared sources in M5 and M8.
The previous commit wrote the sync-and-verify mechanism in the present tense, but no sync target, no verification job, and no fixture exist yet. It also pointed at "the port plan's D10", which is not in the repository at all, so a contributor had no way to reach either the machinery or the reference. Marks the section as intent and says plainly that none of it exists yet. Keeps the rule itself, since that is what the eventual implementation has to satisfy.
The previous commit qualified one section as planned behavior but left the opening asserting that both suites consume fixtures and CI checks them, so the document contradicted itself two paragraphs apart. Rewritten as a whole rather than patched again. It now leads with the status (no fixtures exist, nothing is enforced yet), states the rule as policy, which is true now and independent of any fixture existing, and keeps the sync mechanism explicitly as intent. The root README's one present-tense clause about the R suite reading a synced copy is corrected the same way. Three reviews caught overclaiming in this file because it was written describing an end state rather than the current one.
Three fixes, found by checking each statement rather than patching the one that was reported: - "is where those contracts become executable fixtures that both test suites consume and CI enforces" was still present tense in its main clause. The previous commit corrected only the trailing clause of the same sentence. - The directory table advertised "cross-language spec fixtures" when the directory contains no fixtures. - The CI paragraph claimed the R workflows run on tests/shared/**. Only the three package-check workflows do; deploy filters on pkg-r/** alone and cleanup-previews is unfiltered. This is the same over-generalization already corrected in MIGRATING.md, missed here because that fix was applied to one file rather than to every place the claim appeared. Verified by parsing each workflow's paths filter and comparing it to the text.
Two more inaccuracies in the same sentence: every filter also includes its own workflow file, so "deploy runs on pkg-r/** only" was wrong, and cleanup-previews does not always run, it triggers on closed pull requests and manual dispatch and never on pushes. Both come from restating CI configuration in prose at a precision the prose cannot hold. Replaced the enumeration with the one claim that is stable and worth stating, that a change to one package does not run the other's suite, and a pointer to .github/workflows/ for exact triggers. The workflows are the source of truth for their own behavior.
Collaborator
Author
|
The failing
The cause is #145. Removing the This failure mode arrived with #145. Nothing in this PR can turn that check green, so it needs fixing on
Worth deciding before this merges, since a reviewer otherwise has no way to tell the inherited failure apart from a regression introduced by moving 172 files. |
simonpcouch
approved these changes
Aug 21, 2026
simonpcouch
left a comment
simonpcouch
left a comment
Collaborator
There was a problem hiding this comment.
Nice, looks good! I appreciate you taking care of this as thoughtfully as you have.
Only substantive comment is about README.md; I really have a thing about repo README aesthetics😅😭
Collaborator
There was a problem hiding this comment.
I'm hesitant about this README rewrite—I do think it's important for passersby to see human-written text and the visual demo, and I think we can just make a few tweaks to the existing one to say e.g. "chatlas and ellmer" and "To install, run xyz in R or abc in Python." Fine to copy the existing one into pkg-r and have shared content there, as that content will end up as the index page for the automatically generated site!
Fine with me that the developer-facing content here would really only live in AGENTS.md.
Collaborator
Author
There was a problem hiding this comment.
Worth knowing that this thread is anchored to fc5b099, and I rewrote the README about four hours after you left it, so what you were reading is not what is there now. I think the current version is close to what you are describing, and it would be good to know whether you still object to it.
It keeps the human-written prose, the hex logo, and the screencast, reusing the text from the existing README rather than replacing it. pkg-r/README.md is untouched by the move apart from the install line, so it still works as the pkgdown index page. Developer-facing content is out of it entirely as of 80e6761: the package-directory rule and the MIGRATING.md pointer now live only in AGENTS.md, which I think is the split you were after.
Two of your asks I handled differently, and I would rather explain than quietly not do them.
chatlas is now named next to ellmer, but phrased as "once the Python package is ready" rather than presented as current. Writing it flat as "chatlas and ellmer" reads as though Python support exists, and pkg-py/ is a placeholder pyproject.toml and one placeholder test.
For "to install, run xyz in R or abc in Python", there is no Python command to give: nothing is published to PyPI yet. The R section has the real pak::pak() call and the Python section says the package is in development and not yet installable, with the posit-commons / import commons naming so the eventual shape is visible.
Happy to go further in either direction if you would still rather see the two languages presented symmetrically.
simonpcouch
reviewed
Aug 21, 2026
It was written for a contributor evaluating the restructure: path filters, which workflows trigger on what, why tests/shared/ exists. None of that helps someone who lands here wanting to know what commons is. Now it mirrors pkg-r/README.md in tone and detail, with the logo, the screencast, and the same explanation of the trusted-calculation model, then splits into an R section that tells you what to install today and a Python section that says plainly it is not usable yet. Contributor mechanics are reduced to the one fact that matters, that each package builds from its own directory, plus the pointer to MIGRATING.md. Verified both new links resolve and that Commons is the planned Python entry point.
Three statements read as present-tense Python capability: chatlas provider support, context stored alongside .py files, and parity of layers, tools, and provenance semantics across both languages. None of that exists, and the same README says two sections later that the Python package is not usable. The provider and context sentence is now R-only, matching pkg-r/README.md exactly. The parity claim says outright that only the R package works today and frames the shared design as what the port is being built to. chatlas moves into the Python section, where it reads as intent.
- Drop the explanatory comments from pkg-r/.gitignore. The patterns speak for themselves; the reason the file exists at all is in 0948cc6's commit message and in MIGRATING.md. - Remove CODEOWNERS. Deleting only the /pkg-r/ line, as suggested, would not have had the intended effect, since the `*` catch-all above it routes pkg-r/ regardless. Removing the file is what actually leaves the R side free to take small fixes without an auto-requested review. This departs from the plan's D7; it can come back once the Python package is more than scaffolding. - Move the developer-facing content out of the root README. The package-directory rule and the MIGRATING.md pointer now live only in AGENTS.md. - Name chatlas alongside ellmer in the provider sentence, phrased so it does not claim the Python package works yet, and describe that package as in development and not yet installable.
Collaborator
Author
|
Thanks @simonpcouch. All four points addressed in 80e6761, with details on the individual threads. A couple notes that differed from your suggestions: CODEOWNERS is deleted entirely rather than losing just the The I also merged in latest main. |
Brings in 9d7e0a4 ("skip tests requiring dev ellmer on old ellmer"), which fixes the eight R CMD check failures this branch inherited from #145, plus the two new vignettes and the ellmer static-inspection fix. Merged rather than rebased because the branch is already pushed and reviewed, so rewriting twenty commits would force-push out from under an approval. Three files arrived at paths this branch has moved. Git's directory-rename detection placed the two under vignettes/ correctly and flagged them as conflicts. It could not do the same for pkgdown/extra.css, because this branch never renamed pkgdown/ (the directory did not exist when the move was written), so there was no rename to follow and the file landed silently at the repository root. Moved to pkg-r/pkgdown/extra.css by hand, which is where pkgdown looks for it relative to the package, and which .Rbuildignore already covers with its ^pkgdown$ entry. All three verified byte-identical to origin/main.
simonpcouch
approved these changes
Aug 22, 2026
Comment on lines
+13
to
+15
| ## Two languages, one design | ||
|
|
||
| <img src="https://github.com/user-attachments/assets/67dcf1f2-1496-406a-96cb-50a2e7050eeb" alt="A screencast demonstrating a commons data agent answering questions with a trusted calculation and then a direct data query. In the first case, there's a provenance pill that marks the answer as verified. In the second case, the pill reads 'Untrusted.'" width="100%" /> | ||
| This repository holds both implementations of commons. Today only the R package works. The Python package is being built to the same design, with the same three layers (data, semantic, and context), the same tools, and the same provenance semantics, so that an agent will eventually behave the same way whichever language you build it in. |
Collaborator
There was a problem hiding this comment.
I think we can essentially write the README aspirationally? Since there's an experimental badge, I think we're covered in just writing that things work and getting there eventually.
Fine with me if you decide to ignore here!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the R package from the repository root into
pkg-r/, adds apkg-py/skeleton, and establishestests/shared/for cross-language spec fixtures. Implements kataz26z/ M1 Task 0 of the Python port plan (decision D7).Includes three kata-config commits alongside the restructure, adding
.kata.toml, a kata section inAGENTS.md, and one.gitignoreentry. They are independent of the move and touch three files in total.Why one repository
commons' behavior lives substantially in artifacts that have to agree across both languages: the system prompt, the citation dialect and its guards, the provenance truth table and its display copy, and the tracing span contract. In separate repositories each of those is a copy that drifts silently, and prompt drift fails invisibly, since nothing errors and the agent simply behaves differently in one language.
tests/shared/is where those contracts become fixtures CI enforces instead of prose that rots. The precedent is posit-dev/shinychat.Reviewing this
The first commit is a pure relocation, verifiable rather than taken on trust:
git show --stat -Mreports172 files changed, 0 insertions(+), 0 deletions(-). The remaining commits are each a single concern (CI scoping, install instructions, root scaffolding, docs), so reviewing commit by commit is much easier than reading the squashed diff.The repository is knowingly broken at the first commit, since the workflows still point at root-relative paths until the third.
Things a reviewer could not infer from the diff
.gitignorepatterns. A pattern containing an internal separator is anchored to the directory of the file declaring it, soinst/resources/,inst/tiles/,inst/pkgs,inst/hex/outputandsrc/*.{o,so,dll}all stopped matching once the package moved. The failure is quiet: compiled objects and generated asset directories just become committable. Fixed by splitting package-relative patterns intopkg-r/.gitignoreand leaving only separator-free patterns at the root, where they match at any depth and so coverpkg-py/too.*.Rcheck/and*.tar.gzare also now ignored, which they never were before the split.deploy.ymlpushedpath: instto Connect and would have kept reporting success while deploying a path that no longer exists; it now usespkg-r/instand has apaths:filter so a Python-only PR cannot trigger an R deploy.citation-browser.yamlrunsdevtools::test()and needed the same working-directory treatment asR CMD check.defaults.run.working-directorydoes not apply touses:steps, so the r-lib actions take their ownworking-directoryinput.R-CMD-checkhas norun:steps and therefore no job default at all. pkgdown's deploy step keepsfolder: pkg-r/docsspelled from the repository root, because the JamesIves action resolves it there rather than from the job's working directory.NEWS.mddoes not exist, despite being named in the issue's move list.subdirsweep was two occurrences, not the wider sweep across vignettes andinst/skills/the issue anticipated. Spelled as pak's path formposit-dev/commons/pkg-rrather thansubdir = "pkg-r", which isremotes::install_github()'s argument.Deliberate departures
pkg-py/is not empty. The task asks for py-check green against an empty directory, which is not possible:uv syncneeds apyproject.tomlandpytestexits 5 on a suite with no tests. It carries a placeholderpyproject.toml(package = false) and one placeholder test, both of which the scaffold task deletes. A workflow green because it does nothing would be worse than no workflow.LICENSEandLICENSE.mdare duplicated at the root and inpkg-r/.R CMD checkresolves theDESCRIPTIONreference inside the package, and GitHub only detects a repository license at the root.Needs a repository admin, not a code change
paths:filters mean a required status check that never runs stays pending forever under branch protection and blocks the merge. If any R check is currently required, that setting needs adjusting before this lands.Verification
py-check verified locally:
uv sync --locked,ruff,pytestall pass.R CMD buildfrompkg-r/produces a valid tarball and the structural checks the move could have broken (DESCRIPTIONlocated, namespace information) pass.R CMD checkand the pkgdown build could not be completed locally, since none of the package's twelve hard dependencies are installed in the available R library. That is the one item this PR cannot self-certify, and CI covers it.This branch was rebased onto three commits that landed upstream after it was written (
#132,#138,#145). Those added five new files underR/andtests/testthat/, which git's directory-rename detection folded into the relocation commit; all 27 files those commits touched are byte-identical tomaininsidepkg-r/.Not included
tests/shared/holds only the contract describing what belongs there. Fixtures land as the provenance and citation code is ported, together with the R-side runner. The Python package itself is the next task.Shared shipped artifacts are also out of scope here and decided separately as plan D10: the system prompt moves to a root
prompts/source in M5 and the browser assets to a rootwww/in M8, each synced into both packages with CI failing on a stale copy. Worth knowing because neither an R package nor a Python wheel can ship a file from outside its own directory, so sharing always means a generated copy rather than a direct read. That is also whytests/shared/README.mddescribes the R suite as consuming a synced copy.MIGRATING.mdcovers what a developer used to R tooling at the repository root needs to change. It should be removed at a later date once we're all comfortable with the monorepo.