feat: record review-gated project learnings - #36
Conversation
📝 WalkthroughWalkthroughThe PR adds learning events to trajectories, validates and exports their data model, introduces ChangesProject learning events
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant learningCommand
participant FileStorage
participant addLearning
participant showCommand
User->>learningCommand: trail learning summary and options
learningCommand->>FileStorage: load active trajectory
learningCommand->>addLearning: append learning
addLearning-->>learningCommand: updated trajectory
learningCommand->>FileStorage: save trajectory
User->>showCommand: trail show id --learnings
showCommand->>FileStorage: load trajectory
showCommand-->>User: render project learnings
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/trajectory.ts (1)
208-232: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPersist
validation.data, not the rawlearninginput.
addEventis built from the rawlearningobject rather thanvalidation.data. Sincez.object()strips unrecognized keys by default, any unexpected extra properties onlearningare currently smuggled intorawunchanged, and ifLearningSchemaever adds a.transform()/.default()/trim, this code would silently ignore it.♻️ Use validated data
const validation = LearningSchema.safeParse(learning); if (!validation.success) { const firstError = validation.error.issues[0]; throw new TrajectoryError( firstError.message, "VALIDATION_ERROR", "Check the learning fields and try again", ); } + const learningData = validation.data; return addEvent(trajectory, { type: "learning", - content: learning.summary, - raw: learning, + content: learningData.summary, + raw: learningData, significance: - learning.promotionStatus === "pending_review" ? "high" : "medium", + learningData.promotionStatus === "pending_review" ? "high" : "medium", tags: [ - `learning-source:${learning.source}`, - `learning-area:${learning.area}`, - `promotion:${learning.promotionStatus}`, - ...(learning.recurrenceKey - ? [`recurrence:${learning.recurrenceKey}`] + `learning-source:${learningData.source}`, + `learning-area:${learningData.area}`, + `promotion:${learningData.promotionStatus}`, + ...(learningData.recurrenceKey + ? [`recurrence:${learningData.recurrenceKey}`] : []), ], });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/trajectory.ts` around lines 208 - 232, Update the validated learning flow to use validation.data when constructing the addEvent payload, including content, raw, significance, and tags. Keep the existing validation error behavior unchanged, and ensure all persisted fields reflect the schema-normalized data rather than the original learning input.
🤖 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.
Nitpick comments:
In `@src/core/trajectory.ts`:
- Around line 208-232: Update the validated learning flow to use validation.data
when constructing the addEvent payload, including content, raw, significance,
and tags. Keep the existing validation error behavior unchanged, and ensure all
persisted fields reflect the schema-normalized data rather than the original
learning input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d584465-6364-4281-bb7d-36ce8539c521
📒 Files selected for processing (12)
README.mdsrc/cli/commands/index.tssrc/cli/commands/learning.tssrc/cli/commands/show.tssrc/core/index.tssrc/core/schema.tssrc/core/trajectory.tssrc/core/types.d.tssrc/core/types.tssrc/index.tstests/cli/commands.test.tstests/core/trajectory.test.ts
|
Addressed the validated-data review finding in
Validation: focused 26/26 tests, full 264/264 tests, build, lint, typecheck, and |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/core/trajectory.test.ts (2)
371-378: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the specific validation error.
toThrow()could pass for an unrelated runtime failure. Assert theTrajectoryErrorvalidation contract (or its exposed error code) so this test proves the unsupported promotion status is rejected by schema validation. This follows the downstream contract insrc/core/trajectory.ts:204-234.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/trajectory.test.ts` around lines 371 - 378, Update the addLearning validation test in trajectory.test.ts to assert the specific TrajectoryError validation contract, or its exposed error code, rather than only checking that some error is thrown. Ensure the assertion verifies that the unsupported promotionStatus value is rejected by schema validation.
340-343: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the complete persisted learning payload.
The current partial assertions can pass even if valid fields such as
summary,source, orareaare dropped. Compareevent.rawwith the exact schema-normalized object while also verifying thatunexpectedis absent. The implementation persistsvalidation.datadirectly asraw(src/core/trajectory.ts:204-234).Also applies to: 361-363
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/trajectory.test.ts` around lines 340 - 343, Update the assertions around the persisted learning event in the trajectory test to compare event.raw against the complete schema-normalized validation payload, including fields such as summary, source, area, promotionStatus, and recurrenceKey. Explicitly verify that the unexpected field is absent, while preserving the existing persisted validation.data behavior in the trajectory implementation.
🤖 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.
Nitpick comments:
In `@tests/core/trajectory.test.ts`:
- Around line 371-378: Update the addLearning validation test in
trajectory.test.ts to assert the specific TrajectoryError validation contract,
or its exposed error code, rather than only checking that some error is thrown.
Ensure the assertion verifies that the unsupported promotionStatus value is
rejected by schema validation.
- Around line 340-343: Update the assertions around the persisted learning event
in the trajectory test to compare event.raw against the complete
schema-normalized validation payload, including fields such as summary, source,
area, promotionStatus, and recurrenceKey. Explicitly verify that the unexpected
field is absent, while preserving the existing persisted validation.data
behavior in the trajectory implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e93bea1c-c570-4e40-9a96-bfbc5370ae84
📒 Files selected for processing (2)
src/core/trajectory.tstests/core/trajectory.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/trajectory.ts
1 participant
Summary
Adds the first mergeable slice of #31: structured project learnings that remain separate from decisions, reflections, and durable project instructions.
learningtrajectory event with source, affected area, evidence, recurrence key, and promotion status.trail learningfor recording one-off learnings or human-review candidates.trail show <id> --learningsfor querying them separately.Safety boundary
This slice deliberately does not write to
AGENTS.md,CLAUDE.md, or skills. One-off learnings arearchived;--promotion-candidaterecordspending_review. There is no approved state in this PR, so a candidate cannot be mistaken for a human-approved durable instruction.Recurrence aggregation, configurable thresholds, reviewer authentication, and promotion adapters remain follow-up work.
Validation
npm test -- --run— 263 tests passednpm run buildnpm run lintnpm run typecheckAI assistance
Implemented with OpenAI Codex assistance and reviewed by SuhaibAslam before submission.