Implement attributable RL environments and run lifecycles - #1
Merged
Conversation
Add content-addressed environment registration and immutable-use validation, run provenance snapshots, atomic NDJSON metric logging, ordered series reads, and evidence-rich terminal closure. Enforce 100/100/100 executable coverage, complete declaration docstrings, real Git branch-merge acceptance, npm and Bun lockfiles, fresh-host examples, PM evidence, and maintainer-gated daily publishing.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Summary by CodeRabbit
WalkthroughThe PR adds the ChangesRL extension delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller as CLI caller
participant Extension as pm-rl extension
participant Tracker as pm-cli tracker
Caller->>Extension: register Environment
Extension->>Tracker: validate and persist content-addressed specification
Tracker-->>Extension: return Environment identity
Caller->>Extension: start Run
Extension->>Tracker: persist environment and configuration snapshot
Caller->>Extension: log NDJSON metrics
Extension->>Tracker: append encoded metric events
Caller->>Extension: finish Run
Extension->>Tracker: validate metrics and close Run
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
unbraind
marked this pull request as ready for review
August 1, 2026 12:13
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Owner
Author
|
Review requested after the complete local release gate and fresh installed-host acceptance passed. @greptileai please review the implementation, refusal paths, SDK integration, concurrency proof, and PM evidence. /gemini review @coderabbitai full review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes. |
Reviewer's GuideImplements pm-rl’s first production slice: content-addressed environment specs, attributable run lifecycles with NDJSON metric ingestion into merge-safe notes, strict refusal paths, and full documentation/coverage gates wired into CI and release tooling. Sequence diagram for the new attributable run lifecycle with NDJSON metricssequenceDiagram
actor User
participant pm_cli
participant pm_rl_extension
participant PmClient
participant NotesStore
%% Start run
User->>pm_cli: pm rl run start <id> --environment <env> --algorithm <algo> [--config-file]
pm_cli->>pm_rl_extension: startRun(context)
pm_rl_extension->>PmClient: get(environmentId)
PmClient-->>pm_rl_extension: Environment item
pm_rl_extension->>pm_rl_extension: parseEnvironmentSpec() / hashJson()
pm_rl_extension->>PmClient: create({ type: "Run", dep: [environmentId], affectedVersion: specHash, fixedVersion: configHash, component: algorithm })
PmClient-->>pm_rl_extension: Run item
pm_rl_extension-->>pm_cli: RlCommandResult(action="rl-run-start", id)
%% Log metrics
User->>pm_cli: pm rl run log <id> [--file]
pm_cli->>pm_rl_extension: logRun(context)
pm_rl_extension->>pm_rl_extension: parseNdjsonStream(input)
alt [events.length == 0]
pm_rl_extension->>pm_rl_extension: fail("empty_metric_stream")
else [events.length > 0]
pm_rl_extension->>PmClient: get(id)
PmClient-->>pm_rl_extension: Run item
pm_rl_extension->>PmClient: update(id, { note: events.map(encodeEvent) })
PmClient-->>pm_rl_extension: ok
pm_rl_extension-->>pm_cli: RlCommandResult(action="rl-run-log", appended=events.length)
end
%% Finish run
User->>pm_cli: pm rl run finish <id> --reason <text>
pm_cli->>pm_rl_extension: finishRun(context)
pm_rl_extension->>PmClient: notes(id)
PmClient-->>pm_rl_extension: note texts
pm_rl_extension->>pm_rl_extension: readSeries(note.text[])
alt [series.events.length == 0]
pm_rl_extension->>pm_rl_extension: fail("run_has_no_metrics")
else [series.events.length > 0]
pm_rl_extension->>PmClient: close(id, reason, metadata)
PmClient-->>pm_rl_extension: closed Run
pm_rl_extension-->>pm_cli: RlCommandResult(action="rl-run-finish", metric_events=series.events.length)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Owner
Author
|
Bot response audit (each response read and reacted to):
CI is green on Node 22 and 26. No reviewer-unavailability response is being represented as approval. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 @.agents/pm/features/pm-rl-dyho.toon:
- Around line 15-17: The closure records lack evidence that sustained ingestion
keeps history growth bounded. In .agents/pm/features/pm-rl-dyho.toon lines
15-17, add a sustained-rate ingestion test result with the measured history
bound to actual_result and the acceptance note, or remove/defer the closure
claim; in .agents/pm/history/pm-rl-dyho.jsonl lines 7-8, record matching
bounded-ingestion evidence before the close event. Use the existing
ingestion/history behavior around index.ts batch updates and related tests as
the basis, rather than citing only the 256-event batch.
In @.agents/pm/features/pm-rl-et5b.toon:
- Around line 16-17: Update the feature description to clarify that validation
occurs for extension commands and before environment use, while arbitrary direct
core mutations are not currently rejected at write time. State that host-level
write interception remains pending the required SDK capability, and keep the
existing SDK limitation and tracking context consistent.
In @.github/workflows/release.yml:
- Around line 18-21: Update the release job condition around PM_RELEASE_APPROVED
so it only runs when github.ref equals refs/heads/main, or move the existing
release-ref validation before the Update release version and Commit release
files steps. Ensure feature-branch manual dispatches cannot mutate or commit
release files before being rejected.
In `@index.ts`:
- Around line 213-217: Align the environment ID contract between
registerEnvironment and the registration test so the generated value uses the
expected rl-env- prefix. Replace the direct readFileSync call in
registerEnvironment with the existing readJsonFile helper, preserving the
Environment file context and expected pm CLI error behavior for missing or
unreadable paths.
- Around line 287-297: Update logRun to detect when --file is omitted and stdin
is a TTY before calling readFileSync(path ?? 0, "utf8"). Fail with usage
guidance in that case, while preserving file reads and piped-stdin behavior.
In `@README.md`:
- Around line 62-70: Update the README’s “The two refusals” section to match the
commands currently registered in the Available commands table: document the
implemented empty-run and environment-mutation refusals, or explicitly label
incompatible-version and contaminated-benchmark ranking refusals as planned
behavior. Keep the roadmap and registered-command descriptions consistent.
In `@test/index.test.ts`:
- Around line 175-176: Add a concise comment immediately above the assertion
using logged that documents first_step and last_step reflect event arrival order
from the parsed stream, not numeric step order; leave the assertion and logRun
behavior unchanged.
- Around line 213-222: Update the merge test’s two rl run log invocations for
Agent A and Agent B to use resultOf, retain each command result, and assert that
each operation reports the expected appended count before continuing to the
merge assertions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b2de43a8-1116-4d9e-9fc4-6761f2bb4386
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
.agents/pm/decisions/pm-rl-mpd9.toon.agents/pm/decisions/pm-rl-p401.toon.agents/pm/epics/pm-rl-e20d.toon.agents/pm/features/pm-rl-06n6.toon.agents/pm/features/pm-rl-dpug.toon.agents/pm/features/pm-rl-dyho.toon.agents/pm/features/pm-rl-et5b.toon.agents/pm/features/pm-rl-keet.toon.agents/pm/features/pm-rl-mqdb.toon.agents/pm/features/pm-rl-nzxt.toon.agents/pm/features/pm-rl-taj8.toon.agents/pm/features/pm-rl-tl1x.toon.agents/pm/history/pm-rl-06n6.jsonl.agents/pm/history/pm-rl-dpug.jsonl.agents/pm/history/pm-rl-dyho.jsonl.agents/pm/history/pm-rl-e20d.jsonl.agents/pm/history/pm-rl-et5b.jsonl.agents/pm/history/pm-rl-fpon.jsonl.agents/pm/history/pm-rl-keet.jsonl.agents/pm/history/pm-rl-mpd9.jsonl.agents/pm/history/pm-rl-mqdb.jsonl.agents/pm/history/pm-rl-nzxt.jsonl.agents/pm/history/pm-rl-p401.jsonl.agents/pm/history/pm-rl-taj8.jsonl.agents/pm/history/pm-rl-tl1x.jsonl.agents/pm/tasks/pm-rl-fpon.toon.gitattributes.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdREADME.mdexamples/grid-world.jsonexamples/metrics.ndjsonexamples/ppo.jsonindex.tsmanifest.jsonpackage.jsonscripts/docstring-gate.tsscripts/prepare-merge-driver.tstest/index.test.ts
Owner
Author
|
CodeRabbit’s status was edited from “processing” to a final review limit reached result, so I removed my earlier up-votes and down-voted both CodeRabbit status replies. No CodeRabbit code review ran and there are no findings to resolve. Sourcery likewise reported its weekly quota, Gemini reported service sunset, and Greptile has produced no response. Node 22/26 CI and every local release/installed-host gate remain green; reviewer unavailability is recorded as a limitation, not approval. |
Fail fast for TTY metric input, normalize environment file errors, harden feature-branch release refusal, align current refusal docs, and strengthen merge assertions. Reopen bounded-ingestion PM work because the existing 256-event proof does not establish long-run retention bounds; clarify the environment write-enforcement boundary.
Owner
Author
|
Review round 1 complete: all eight CodeRabbit comments were read, up-voted for actionable value, and answered in their threads. Seven were implemented directly; the environment-id prefix sub-finding was rejected with host-prefix evidence while its valid raw-file-error half was fixed. The unsupported bounded-retention closure was corrected by reopening pm-rl-dyho rather than manufacturing evidence. After commit 934dff0: @greptileai please review the new commit and the resolved threads. @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 43 minutes. |
Owner
Author
|
Final thread audit used the thread-aware GraphQL workflow: 8/8 review threads are resolved, with no unresolved or outdated actionable thread. I also read and up-voted every CodeRabbit confirmation reply; the bot explicitly accepted the tracker-prefix explanation and added it as a learning. The edited summary and completed first-review acknowledgement are now up-voted; the separate follow-up-review quota response is down-voted because no second full scan ran. The follow-up commit nevertheless has fresh green Node 22/26 CI and a complete local release gate. |
This was referenced Aug 1, 2026
This was referenced Aug 9, 2026
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
1 participant
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.
Outcome
Implements pm-rl’s first production slice on pm CLI/SDK 2026.8.1: content-addressed environments, exact run provenance, atomic NDJSON metric ingestion, ordered metric reads, and evidence-rich completion that refuses empty runs.
Evidence
pm-opsgatenpm ci, zero production vulnerabilities, deterministic pack/changelog checks, and Bun install/lock verificationPM_RELEASE_APPROVEDrepository variable; this PR does not publish the new npm packagePM records
Upstream SDK findings
pm-rl-et5bintentionally remains in progress for #854; commands already detect an out-of-band environment mutation before use and snapshot exact environment content into every Run.Summary by Sourcery
Introduce the first production slice of pm-rl with content-addressed RL environments and attributable run lifecycles that ingest and expose merge-safe metric streams via pm notes.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Summary by cubic
Add content-addressed RL environments and an attributable run lifecycle with NDJSON metric ingestion into merge-safe notes. Adds fail-fast TTY protection for
run log, normalizes environment file errors, strengthens the real Git merge proof, and documents current retention bounds and write-enforcement limits.New Features
env list/showadded; referenced versions are immutable and verified before each run; file errors are normalized.pm rl run start/log/finish/verify: start snapshots algorithm, hyperparameters, checkpoint, and environment ID; log validates NDJSON, refuses TTY stdin, and appends as notes; finish records terminal metrics and refuses empty runs.examples/grid-world.json,examples/ppo.json,examples/metrics.ndjson.Dependencies
@unbrained/pm-cli >= 2026.8.1and Node >= 22.18.0.pm-opsdocstring gate and enforces 100/100/100 coverage; includespm-changelog.PM_RELEASE_APPROVEDis true and the branch ismain.bun.lockandpackage-lock.json; release remains gated byPM_RELEASE_APPROVED.Written for commit 934dff0. Summary will update on new commits.