fix(provider): validate native NaN login and retain offline models (2/2) - #1571
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughNaN now registers as a native Pi provider with API-key login, credential resolution, host-native streaming, and a credential-scoped model catalog. Its documented offline fallback includes seven chat models. Successful discovery publishes the current catalog, including an empty result. ChangesNaN provider integration
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Pi
participant NaNProvider
participant NaNModelDiscoveryAPI
Pi->>NaNProvider: Refresh provider models
NaNProvider->>NaNModelDiscoveryAPI: Fetch models using current credentials
NaNModelDiscoveryAPI-->>NaNProvider: Return discovered models
NaNProvider-->>Pi: Publish current catalog update
Suggested reviewers: Merge Risk: ⚪ Minimal · up to NaN becomes a native Pi provider with validated API-key login and a seven-model offline fallback. The reported checks passed and no merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Credential validation, fixed destinations, and cancellation controls limit the risk. The remaining uncertainty is whether authentication and catalog refresh consistently use the same account identity, particularly with environment-provided credentials. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1 unsupported.)
✨ 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 |
Closes #1569
Depends on #1570. This PR targets its parent branch to keep the review at 307 changed lines. Land #1570 first, then retarget/rebase this PR to
main; do not merge it into the parent feature branch.PR type
Summary
Changes
extensions/nan-provider.tslib/nan-provider.tstests/nan-provider.test.tstests/runtime-harness.mjsREADME.mdTest plan
node --experimental-strip-types --test tests/nan-provider.test.ts: 20 passed.pnpm run typecheck: baseline gate passed, 187 recorded diagnostics, no regressions.env -u GENTLE_PI_AGENTS_CHILD -u GENTLE_PI_CONFIG_HOME pnpm test: 4,054 passed, 50 skipped, zero failures; provider-contract/runtime-harness passed.git diff --check: passed.lib/nan-provider.ts:114-118is non-blocking and opened no correction.Earlier same-source validation covered eight isolated Pi 0.87.1 direct/picker login cases with synthetic credentials, seven available offline models with no network requests, and one successful authenticated
nan/deepseek-v4-flashinference. The user also confirmed model selection works. Those are prior/manual results, not claims of fresh live calls in this delivery run.The offline catalog declares documented model support, not account entitlement. A successful live catalog remains authoritative. MCP search/media bridges are not implemented.
Contributor checklist
type:*label:type:bug.Chain context
feat/nan-provider-01-connectionat282cce68Includes: native authentication, publication/offline regressions, tests and documentation. Excludes: MCP tools, usage-layer changes, personal launcher/configuration and unrelated provider changes. Both slices were tested independently; reverting this slice restores the initial connection without removing it. No auto-merge is requested.
Summary by CodeRabbit
/loginor/login nan. API keys are trimmed before saving; blank input is rejected, and cancelling leaves the saved key unchanged. Saved keys take precedence overNAN_API_KEY.