Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: juspay/neurolink/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (143)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughAdds Morph and Novita to the provider catalog, exported model and credential types, routing checks, and OpenAI-compatible capability tests. ChangesProvider integration
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant CatalogTest
participant NeuroLink
participant ProviderEndpoint
CatalogTest->>NeuroLink: Call generate() with provider credentials and model
NeuroLink->>ProviderEndpoint: Send OpenAI-compatible request
ProviderEndpoint-->>NeuroLink: Return response
NeuroLink-->>CatalogTest: Return parsed output or tool result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
Documentation Validation Results🚀 Documentation validation passed!
📦 Build artifact uploaded successfully. Ready for deployment preview. Commit: |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Novita catalog onboarding review — 2 inline findings (1 MINOR text-consistency, 1 question). See inline comments.
|
Superseded — retained for history only. The canonical single review summary for this PR is the most recent |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Novita catalog onboarding reviewed. Verdict: APPROVE (with minor nits) — well-executed additive catalog entry with evidence-backed capability flags and a sound default-model choice. One MINOR text-consistency finding on the meta-llama/llama-3.3-70b-instruct description (suggestion block posted); the alias-check concern was resolved by the harness code (see thread).
438a47e to
ed6fbd3
Compare
|
Superseded — retained for history only. The canonical single review summary for this PR is the most recent |
Live acceptance run — SDK and CLI, real vendorThe body described live probes in prose; this is the output. Run from this PR's worktree at head All four surfaces: The probe writes its transcript to a file and calls |
ed6fbd3 to
63a8a3c
Compare
Historical record — was head
|
63a8a3c to
d3d6557
Compare
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Recurring review pass — verified the Novita onboarding (prior findings remain resolved) and reviewed the Morph catalog commit (head, 9c2f0c5) which was not covered by the earlier approved pass. Two MINOR findings; nothing blocking.
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Recurring pass. Novita findings remain resolved. Morph (head commit) reviewed clean. Two MINOR nits attached inline — nothing blocking.
9c2f0c5 to
ca731b0
Compare
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Recurring-pass verdict: APPROVE on the squashed head ca731b03.
Both onboarded providers (Novita, plus Morph added atop) are clean, additive catalog onboarding under a legitimate dual-provider scope. The morph-scope-mismatch governance note is resolved by the squash; morph-novita-e2e-coverage-gap remains a tracked, non-blocking MINOR (Rule 15). Nothing blocks merge.
ca731b0 to
d7d0816
Compare
murdore
changed the title
|
Force-pushed What was wrong#1648 (Morph) was merged into this branch rather than into release, so this PR now carries both providers — that part is fine and the title is updated to match. But the rebase that produced
Why nothing caught itThis is the part worth internalising, because it is the third occurrence today. The bad head reported
The only signal was the diffstat: 54 files and 2,056 deletions for what should add two JSON files. What I didReset to the current release tip, dropped #1657 (FriendliAI) is stacked on this branch and inherited the same bad tree. It needs the identical treatment after this lands — I'll do it then, since its base moves when this merges. |
Recurring review — head unchanged (historical record for head
|
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Recurring-pass verdict on the current head d7d0816b (APPROVE), submitted as an approving review so the PR's review state matches the canonical <!-- yama:summary --> verdict.
Both providers (Novita + Morph) remain clean, additive catalog onboards. One non-blocking MINOR (Rule 15 E2E coverage, thread morph-novita-e2e-coverage-gap) stays open as a tracked follow-up; all other findings resolved. Nothing blocks from a review standpoint — branch is blocked on CI only.
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Recurring-pass verdict: APPROVE on the current head d7d0816b2e51.
The Novita + Morph catalog onboarding is clean and additive. All prior findings are either resolved (novita-default-description, novita-aliascheck-model, morph-scope-mismatch) or a tracked, non-blocking MINOR (morph-novita-e2e-coverage-gap). Nothing blocks merge.
|
@murdore — confirmed and re-verified this head. I independently re-checked the three changed catalog files ( The approval stands on Agreed that #1657 (FriendliAI) inherits the same bad tree and needs the identical regenerate-from-release after this lands. |
d7d0816 to
1f1297a
Compare
|
Superseded — retained for history only. The canonical single review summary for this PR is the most recent |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
APPROVE. Clean, additive catalog onboarding of Novita and Morph; all four review findings resolved at head 1f1297a2. Registry pattern, backward compatibility, security and mock-level coverage all verified clean. See the PR's <!-- yama:summary --> comment for the full record.
1f1297a to
3d7a979
Compare
|
Superseded — retained for history only. This pass's summary is folded into the canonical single review summary for this PR, the |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
APPROVE. Clean, additive catalog onboarding of Novita and Morph; all four review findings resolved at the current head. Registry pattern, backward compatibility (additive enum/credential/model members), security and mock-level coverage all verified clean at head 3d7a9797. Submitted as an approving review on the current head so the PR's review state matches the canonical <!-- yama:summary --> verdict. See that summary comment for the full record.
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
APPROVE.
Final validation pass at current head 3d7a9797. All four findings remain resolved (novita-default-description, novita-aliascheck-model, morph-scope-mismatch, morph-novita-e2e-coverage-gap — the last a tracked non-blocking MINOR, Rule 15). The Novita + Morph catalog onboarding is clean, additive, and sibling-conformant.
Review-state reconciliation complete:
- One canonical summary comment (
<!-- yama:summary -->) with verdict APPROVE. - One comment per finding (all resolved in-thread) — no duplicates.
- No malformed comments or broken suggestion blocks.
- The non-marker legacy summary was explicitly superseded for history.
Matching the summarised verdict, this PR's review state is now an approving review. Nothing blocks from a review standpoint; the branch is blocked on CI only.
See the PR's <!-- yama:summary --> comment for the full record.
3d7a979 to
7b50ce2
Compare
Recurring review — follow-up on head
|
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
APPROVE.
Follow-up delta at current head 7b50ce28 verified clean: live-matrix.yml now wires NOVITA_API_KEY/MORPH_API_KEY (self-gating until secrets exist), docs/api/** is regen churn, and the rebase kept both FriendliAI and this PR's test sections. All four findings remain resolved; catalog content and capability cases unchanged and sound. Submitted as an approving review on the current head so the PR's review state matches the canonical <!-- yama:summary --> verdict. See that summary comment for the full record.
7b50ce2 to
5386c11
Compare
Historical record — recurring pass at head
|
|
Closing and reopening to re-run the pull_request workflows against the new release (#1763 landed the reproducible search-index generator), without a force-push. |
|
Recurring review — Novita & Morph catalog onboarding Result: APPROVE (no new findings; supersedes #5824881504) This pass re-validates the current head
The PR body's claim — "no source or test change, pure rebase and regen" — is verified. All four prior findings remain resolved and hold against the current head:
Verification this pass
No inline findings to post this pass. Nothing blocks from a review standpoint; the branch is blocked on required CI only. |
|
@murdore — acknowledged on the close/reopen to re-run
Nothing blocks from a code-review standpoint; the baton is with the CI gate / merge. |
Two Tier-2 catalog onboardings: Novita (api.novita.ai) and Morph (api.morphllm.com), both OpenAI-wire-compatible. Two catalog JSONs, the generated regions they drive, and three alias rows in the catalog suite (novita, morph, morphllm). Rebuilt by regenerating rather than by resolving conflicts. The previous head resolved its rebase the other way — taking the branch's own side wholesale — and silently reverted 2056 lines of src/test plus 236 lines of config and docs that had landed on release in between: test/continuous-test-suite-tools-manager-truncation.ts -568 (#1622) test/continuous-test-suite-native-vendor-recovery.ts -443 src/lib/context/nativeGenerateGuard.ts -234 (#1668) test/continuous-test-suite-reasoning-parity-live.ts -198 (#1673) src/lib/core/modules/ToolsManager.ts -248 (#1622) test/helpers/mockChatServer.ts -155 CLAUDE.md -156 (#1676, #1687) plus toolOutputLimits, nativeGenerateLoop, live-matrix.yml, eslint.config.js None of that was intended by a catalog onboarding, and none of it is touched here. Nothing detected it either: the head reported mergeable/clean, zero conflicts, 5/5 required checks and a single valid commit, because deleting a test suite does not fail a test run and a deletion is not a conflict. The only signal was the diffstat. This commit takes the two catalog JSONs unchanged, replays them on the current release tip, and re-runs `pnpm run codegen:catalog` so the enum members, the credentials key and the catalog index all derive from the JSON. There is nothing to hand-resolve, which is what removes the failure mode rather than merely avoiding it this time. Review follow-up: the catalog suite's alias-routing case only proved a plain "ping" reaches each host, not the capability claims in the catalog entries themselves. Added four more cases: Morph (messageContentFormat quirk) - an image is rejected client-side (0 HTTP calls) before Morph's wire-format coercion could ever run, because morph.json declares vision:false on every model - ordinary multi-turn chat (conversationMessages + a new turn) always sends string content end to end The literal "send an array, watch it coerce to a string" request is not reachable through generate()/stream() for Morph specifically — its vision:false models block the one path that builds array content, and its tools:false capability blocks the other (a tool_calls round-trip, the mechanism the Cloudflare messageContentFormat test in continuous-test-suite-providers-mocked.ts relies on). Both cases say so in the section header instead of fabricating an unreachable request. Novita (structuredOutput + tool-calling) - schema-bound generate() asserts the request carries response_format.json_schema and result.structuredData is populated - a tool-calling round-trip asserts the first request actually offers `tools`, the follow-up turn replays the assistant's tool_calls plus the executed tool's result message, and result.toolsUsed/content reflect it — mirroring novita.json's own evidence.liveMatrix proof (getTime(tz=Asia/Tokyo) -> {"city":"Tokyo","time":"09:00"}) Verified both are non-vacuous by temporarily breaking the real behavior: flipping capabilities.tools/structuredOutputWithTools to false in novita.json fails the tool-calling case, and flipping vision to true in morph.json fails the image-rejection case. Both catalog files were restored byte-for-byte afterward (git diff clean) and the suite rebuilt green. Verified: codegen:catalog --check exit 0 prettier --check exit 0 check (svelte-check + tsc) exit 0 lint exit 0 (83 pre-existing warnings, unrelated) build exit 0 continuous-test-suite-openai-compat-catalog.ts 50 passed · 0 failed (was 46; +4 new cases: 2 Morph, 2 Novita) Source diff against release: 8 files, +238 src, +366 test, +2 workflow, zero deletions. Live matrix: NOVITA_API_KEY and MORPH_API_KEY are wired into .github/workflows/live-matrix.yml's nightly sweep the same way FRIENDLI_API_KEY is. Neither secret exists in the repository yet, so the sweep self-gates on both providers until the secrets are added.
5386c11 to
3d857eb
Compare
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
APPROVE.
Re-validation of the current head 3d857eb21 (pure rebase + codegen:catalog regen over the previously approved 5386c117) is clean. The reviewed catalog content is byte-identical across both SHAs: novita.json, morph.json, and live-matrix.yml carry the same blobs; enums.ts/providers.ts differ only by base drift with the PR's own additive members intact. All four prior findings remain resolved; registry pattern, backward compatibility, security, and the two capability-regression cases all verify clean. Submitted as an approving review on the current head so the PR's review state matches the canonical <!-- yama:summary --> verdict. See that summary comment for the full record.
This branch has not been deployed
2 participants
Summary
Onboards Novita and Morph as Tier-2 OpenAI-compatible catalog providers.
Novita's payment wall cleared, so its wire contract could be recorded end to end and turned into a catalog entry. Everything here comes from that recording, not from vendor documentation. Morph — a Fast-Apply code-editing specialist, not a general chat vendor — was recorded the same way and added alongside it.
Recorded at the wire (Novita): roster 200 with 156 model ids; chat, system role, array content parts, sampling params and
max_completion_tokensall accepted; SSE streaming with a usage chunk and[DONE]; a real structured tool call;json_objectandjson_schemaaccepted; 401failed to authenticate API key; 404model not found.Recorded at the wire (Morph): roster 200 (22 ids, only 2 confirmed servable at
/chat/completions); chat,max_completion_tokens, system role, sampling params, SSE streaming,json_schema/json_objectall 200. Array-shaped message content is rejected with HTTP 500 (text.charCodeAt is not a function) — the same class of quirk as Cloudflare (#1587) — soquirks.messageContentFormatis"string"and no model is markedvision:true. Tool calling returns HTTP 200 but doesn't actually work:morph-v3-largeechoes the call back as literal<tool_call>...</tool_call>text instead of atool_callsarray, andmorph-v3-fastignorestoolsentirely — sotools/toolsWithStreamingarefalsedespite the 200s.The Novita default is deliberately not the plain instruct model
meta-llama/llama-3.3-70b-instructreturns HTTP 400 for both structured-output formats:zai-org/glm-5.3-flashreturns 200 forjson_object,json_schemaand tools-plus-schema together. The catalog schema expressesstructuredOutputper provider, not per model, so making the instruct model the default would have declared a capability the default caller could never use. The default is thereforezai-org/glm-5.3-flash, which is also Novita's own served default; the instruct model stays as a fallback for plain chat.Capability flags and their evidence
tools/toolsWithStreamingfinish_reason=tool_callswith a structuredget_timecall.structuredOutputjson_objectandjson_schemaboth 200 on the default model.structuredOutputWithToolsjson_schemaaccepted together, 200; the combined follow-up turn (model must answer the original schema question from a tool result) was proved separately throughgenerate({ schema, tools })end to end viadist/index.js.tools/toolsWithStreaming<tool_call>text onmorph-v3-large, ignored onmorph-v3-fast).structuredOutputjson_schema/json_objectboth return schema-conformant JSON.structuredOutputWithToolstools:false— a 200 to tools+schema together isn't evidence when tools don't work.Live proof against the built package
Run from
dist/, not source: Novita generate on the default model returnsReady; streaming yields 7 chunks1, 2, 3; a schema call returns{"colour":"blue"}withstructuredDataparsed. Default resolution was re-checked after a rebuild, because the first check read a staledist/and silently resolved the old model.Changes
src/lib/providers/catalog/novita.json,src/lib/providers/catalog/morph.json— new catalog entries.src/lib/constants/enums.ts—NOVITA/MORPHinAIProviderName+NovitaModels/MorphModelsenums.src/lib/types/providers.ts—novita?/morph?credential keys (additive).src/lib/providers/catalog/index.generated.ts,src/lib/types/providerCatalog.generated.ts— catalog registry (generated).test/continuous-test-suite-openai-compat-catalog.ts— alias-routing rows (novita,morph,morphllm) plus two capability-regression sections:testMorphContentFormatSection(image rejected client-side before any HTTP call; multi-turn chat always sends string content) andtestNovitaCapabilitiesSection(schema-boundgenerate()assertsresponse_format.json_schema+ populatedstructuredData; agetTimetool-calling round-trip assertstoolsis offered and the result is replayed).docs/api/**— generated line-number churn required by the drift gate.Known, and not introduced here
A schema request through the SDK returns prose with
structuredData: nullon already-shipped catalog providers too, including Groq and Cerebras. That is a pre-existing gap in the shared structured-output path, filed separately rather than worked around here.Testing evidence
Refreshed onto
releasea7c82e821after #1781, #1794 and #1795 landed: the non-generated diff reproduced byte-identical (patch-id9324a6a49ea1),docs/apiwas regenerated, andsearch-index.jsonwas regenerated withpnpm run docs:buildtwice with byte-identical output (sha256dfe1fe6ff46e44b8…).pnpm run codegen:catalogregenerated the generated catalog regions, andtest:provider-wiringpasses 26/26. New head3d857eb21. No source or test change.Head sha:
3d857eb21c4a97097b66144f8ef62f479751fdac(this commit) · Release sha at rebase time:75db63d41c58cf2f121cb51590e0e20f3c13c2ca(git rev-parse origin/release).Rebase.
rebase-stage.shreportedSTAGED_CONFLICTS test/continuous-test-suite-openai-compat-catalog.ts— release had landed FriendliAI (#1657), which added atestCatalogFallbackRule()call at the same spot inmain()that this PR's owntestMorphContentFormatSection()/testNovitaCapabilitiesSection()calls occupy. Resolved by keeping all three calls, then ranpnpm run codegen:catalog(idempotent — a second run produced no diff) so the generated regions carryfriendli,novitaandmorphtogether.NOVITA_API_KEY / MORPH_API_KEY wiring. This PR previously claimed
NOVITA_API_KEYwas wired into the nightly live matrix, but the diff never touched.github/workflows/live-matrix.yml. Fixed: addedNOVITA_API_KEYandMORPH_API_KEYto that workflow's "Live provider matrix sweep" step, the same wayFRIENDLI_API_KEYis wired there. Checked withgh secret list --repo juspay/neurolinkandgh api repos/juspay/neurolink/actions/organization-secrets: neither secret exists yet (29 repo secrets + 2 org secrets, neitherNOVITA_API_KEYnorMORPH_API_KEYnor, notably,FRIENDLI_API_KEYamong them) — the sweep is designed to self-gate cleanly when a key is absent, so this entry joins most of the other 40-odd providers already wired there in that same state until the secrets are added.Commands and results (run against the committed HEAD in this worktree, post-commit; logs under
proof/):pnpm run buildpublint: All good!pnpm exec tsx test/continuous-test-suite-openai-compat-catalog.tscapabilities.tools/structuredOutputWithToolsflipped tofalseinnovita.jsonandvisionflipped totrueinmorph.json(working tree only, not committed), rebuiltgit checkout HEAD -- ., rebuiltFixed / restored summary line (identical in both logs):
Broken-on-purpose: the two targeted failures (53 passed · 2 failed overall), each an actual
✗with anon-zero exit, not a skip:
No
⊘skips appear in any of the three runs. Full logs:proof/continuous-test-suite-openai-compat-catalog.{fixed,broken,restored}.log, exact revert inproof/continuous-test-suite-openai-compat-catalog.revert.md.Review follow-ups
All 4 review threads were already resolved before this pass; re-verified against the current head, all still hold:
novita-default-description(MINOR) — catalog text said the Llama instruct model was "chosen as the runtime default", contradictingmodels.default. Fixed:novita.json's Llama description now reads "...the runtime/default model is zai-org/glm-5.3-flash".novita-aliascheck-model(question) — alias check pins the instruct model, not the catalog default. No change needed: confirmed the harness passes the literalcheck.modelstraight intogenerate()for plain-chat routing only; no reliance on the catalog default.morph-scope-mismatch(MINOR, governance) — PR title/scope originally covered only Novita, then Morph was added on top. Fixed: PR retitled to "onboard Novita and Morph as catalog providers", matching the actual diff.morph-novita-e2e-coverage-gap(MINOR, Rule 15) — the capability claims (Novita structured-output/tools, Morph's array-content quirk) weren't regression-guarded by any e2e case. Fixed: addedtestMorphContentFormatSection()andtestNovitaCapabilitiesSection(); both are non-vacuous (verified by temporarily flipping the underlying catalog flags and watching the corresponding case fail, then restoring).Latest Yama (Tara-ag) review: APPROVE — all four findings resolved, nothing blocks from a review standpoint. No actionable items from CodeRabbit's comment (release-notes summary only; no nitpick/outside-diff/actionable sections).
Summary by CodeRabbit
New Features
Bug Fixes