Repository navigation
refactor: define THINKING_LEVELS in one place - #432
amandeavor wants to merge 2 commits into
Conversation
Import the canonical list from model-stream into protocol provider-management, and build the catalog-validation Set from the @axl/protocol export so level renames cannot drift. Closes Observal#422
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Use shared thinking-level list packages/protocol/src/provider-management.ts, packages/ai/src/catalog-validation.ts |
Provider-management imports the shared constant instead of defining a local list. Catalog validation builds its validation set from the protocol constant. Existing validation use remains in place. |
Priority:
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Refactor
Suggested reviewers: kaushik-kumar-ceg
Merge Risk: 
7ea56
-
The refactor preserves the supported thinking levels and uses the protocol package’s public export. No actionable merge-blocking risk was identified.
🚥 Pre-merge checks | ✅ 3 | ❌ 2
-
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution Description check ⚠️ WarningThe description explains the refactor, references issue #422, and lists protocol and AI test results. It does not follow the repository template and omits the required Purpose, Fixes, Approach, How wa…Update the description to use the repository template. Include the purpose, fixes reference, implementation approach, compatibility or security impact, complete test commands and results, all checklist items, and the AI assistance section. Docstring Coverage ⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. ✅ Passed checks (3 passed)
-
Check name Status Explanation Title check ✅ Passed The title clearly describes the main change: centralizing the THINKING_LEVELS definition. Linked Issues check ✅ Passed Issue #422requires oneTHINKING_LEVELSdefinition, reuse inprovider-management.tsandcatalog-validation.ts, preserved package boundaries, and passing protocol and AI tests. The PR imports `TH…Out of Scope Changes check ✅ Passed The reported changes are limited to replacing duplicate THINKING_LEVELSdefinitions with the canonical export in the two locations named by issue#422. The changes support the requested pure refacto…
Full details: Description check
-
Explanation
The description explains the refactor, references issue
#422, and lists protocol and AI test results. It does not follow the repository template and omits the required Purpose, Fixes, Approach, How was this tested?, Checklist, and AI assistance sections.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Summary
THINKING_LEVELSwas duplicated inpackages/protocol/src/provider-management.tsand as a hand-writtenSetinpackages/ai/src/catalog-validation.ts. Both now consume the canonical export frommodel-stream.ts/@axl/protocol.Closes #422
Test plan
pnpm --filter @axl/protocol test(39 pass)pnpm --filter @axl/ai test(284 pass)Summary by CodeRabbit