Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial PlanningService v0.1 contract and a set of deterministic planning fixtures intended to sit above the existing execution bridge, making planning operations auditable and reference-driven.
Changes:
- Introduces
PlanningServicecontract v0.1 documentation. - Adds planning fixture lane with request/response JSON examples for program induction and counterexample search.
- Adds a fixtures README describing intended fixture coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
docs/contracts/planning-service-v0.1.md |
Defines PlanningService goals, methods, and constraints for governed planning. |
fixtures/planning/README.md |
Describes the purpose and planned coverage of planning fixtures. |
fixtures/planning/induce-program-candidate.request.json |
Example request payload for InduceProgramCandidate. |
fixtures/planning/induce-program-candidate.response.json |
Example response payload for InduceProgramCandidate. |
fixtures/planning/search-counterexample.request.json |
Example request payload for SearchCounterexample. |
fixtures/planning/search-counterexample.response.json |
Example response payload for SearchCounterexample. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `BacktrackPlanNode` request/response | ||
| - `ValidateAbstractRule` request/response | ||
|
|
||
| These fixtures preserve stable refs to scope, state, objective, and plan-node artifacts. |
There was a problem hiding this comment.
This states the fixtures “preserve stable refs to scope, state, objective, and plan-node artifacts”, but the current fixtures in this directory only include scopeRef, planNodeRef/targetRef, etc. Consider either adding the referenced stateRef/objectiveVectorRef fields to the fixtures or adjusting this sentence to match what’s currently present.
| These fixtures preserve stable refs to scope, state, objective, and plan-node artifacts. | |
| These fixtures preserve stable refs to scope, plan-node, and target artifacts. |
| Planning methods MUST operate on stable references to: | ||
| - planning scope ids | ||
| - belief-state refs | ||
| - objective-vector refs | ||
| - policy refs | ||
| - plan-node refs |
There was a problem hiding this comment.
In the “Input discipline” section you say planning methods must operate on “stable references”, but the first bullet is “planning scope ids” (and others are “refs”). This mixes identifier terms and could confuse implementers about whether values are expected to be ...Id or ...Ref strings. Consider making all bullets consistently *Ref (or consistently *Id) and aligning the wording accordingly.
|
|
||
| ## Rule | ||
|
|
||
| Planning methods SHOULD preserve stable references back to `scopeId`, `stateRef`, `planNodeId`, and `objectiveId`. |
There was a problem hiding this comment.
The “Rule” line mixes scopeId / planNodeId / objectiveId with stateRef, while the rest of the contract emphasizes stable *Ref values. Please standardize the terminology (either all *Ref or all *Id) so the contract is unambiguous about what is passed over the wire.
| Planning methods SHOULD preserve stable references back to `scopeId`, `stateRef`, `planNodeId`, and `objectiveId`. | |
| Planning methods SHOULD preserve stable references back to `scopeRef`, `stateRef`, `planNodeRef`, and `objectiveVectorRef`. |
|
|
||
| The service SHOULD attach one or more of: | ||
| - `programCandidateRef` | ||
| - `counterexampleRef` |
There was a problem hiding this comment.
The contract suggests attaching a counterexampleRef, but the fixture response uses counterexampleRefs (plural array). Please align the contract field naming with the intended wire shape (single ref vs list) to avoid incompatible implementations.
| - `counterexampleRef` | |
| - `counterexampleRefs` |
|
@copilot Fix failed checks |
Agent-Logs-Url: https://github.com/SocioProphet/TriTRPC/sessions/acf5304d-a314-47a6-8fc6-42d645c1688d Co-authored-by: mdheller <21163552+mdheller@users.noreply.github.com>
Implemented fixes for the failed checks in commit |
Agent-Logs-Url: https://github.com/SocioProphet/TriTRPC/sessions/acf5304d-a314-47a6-8fc6-42d645c1688d Co-authored-by: mdheller <21163552+mdheller@users.noreply.github.com>
Agent-Logs-Url: https://github.com/SocioProphet/TriTRPC/sessions/acf5304d-a314-47a6-8fc6-42d645c1688d Co-authored-by: mdheller <21163552+mdheller@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request any any failed tests |
…conflicts Co-authored-by: mdheller <21163552+mdheller@users.noreply.github.com>
Agent-Logs-Url: https://github.com/SocioProphet/TriTRPC/sessions/519dc520-44d3-4eee-9e1e-e1fe2ddb9f20 Co-authored-by: mdheller <21163552+mdheller@users.noreply.github.com>
Done. I merged |
This PR adds a planning-service layer above the existing execution bridge.
Why:
What this PR adds:
PlanningServicecontract v0.1What this PR does not do:
ExecutionBridgeService