Skip to content

Qaa 665 llm add typecheck #13523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cunhabruno wants to merge 2 commits into from
Closed

Qaa 665 llm add typecheck #13523

cunhabruno wants to merge 2 commits into from

Conversation

@cunhabruno
Copy link
Contributor

@cunhabruno cunhabruno commented Jan 5, 2026 •
edited by atlassian bot
Loading

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

📝 Description

  • Add typecheck for e2e mobile tests
  • Add husky pre push hook to run typecheck before push

❓ Context

🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

Copilot AI review requested due to automatic review settings January 5, 2026 08:22
@cunhabruno cunhabruno requested review from a team as code owners January 5, 2026 08:22
@vercel
Copy link

vercel bot commented Jan 5, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments
Project Deployment Review Updated (UTC)
ledger-live-github-bot Ignored Ignored Preview Jan 5, 2026 0:26am
native-ui-storybook Ignored Ignored Preview Jan 5, 2026 0:26am
react-ui-storybook Ignored Ignored Preview Jan 5, 2026 0:26am
web-tools Ignored Ignored Preview Jan 5, 2026 0:26am

@live-github-bot live-github-bot bot added the mobile Has changes in LLM label Jan 5, 2026
@cunhabruno cunhabruno changed the title Qaa 665 llm add type check Qaa 665 llm add typecheck Jan 5, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026 •
edited
Loading

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

🖥️ Desktop

-> Run Desktop E2E

  • Select "Run workflow"
  • Branch: QAA-665-llm-add-type-check
  • Device: nanoSP or stax

📱 Mobile

-> Run Mobile E2E

  • Select "Run workflow"
  • Branch: QAA-665-llm-add-type-check
  • Device: nanoX

@cunhabruno cunhabruno force-pushed the QAA-665-llm-add-type-check branch from fe61633 to 8ba1961 Compare January 5, 2026 08:25
Copilot AI reviewed Jan 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds TypeScript type checking for the e2e mobile test suite and configures a Husky pre-push hook to automatically run type checks before pushing code. The changes address type errors in the e2e tests by updating TypeScript configuration and fixing import paths.

Key changes:

  • Added typecheck npm script and Husky 9.1.7 as a dev dependency
  • Updated TypeScript configuration with modern compiler options and corrected path mappings
  • Fixed import paths and removed unnecessary explicit type annotations that can be inferred

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
e2e/mobile/tsconfig.json Updated TypeScript compiler options with bundler-style settings and changed live-common paths from src to lib directories
e2e/mobile/package.json Added typecheck script, husky dependency, and prepare script for Husky setup
e2e/mobile/.husky/pre-push Added pre-push hook to run typecheck on e2e mobile files
e2e/mobile/.husky/install.mjs Added Husky installation script that skips in production/CI environments
e2e/mobile/utils/initUtil.ts Fixed import path from tilde alias to relative path for type definitions
e2e/mobile/specs/ledgerSync/ledgerSync.spec.ts Removed explicit type annotations in favor of type inference
e2e/mobile/bridge/server.ts Changed WebSocket import from named import to namespace import
e2e/mobile/page/wallet/walletTabNavigator.page.ts Alphabetically sorted imports
e2e/mobile/page/settings/settingsHelp.page.ts Removed blank line
pnpm-lock.yaml Updated dependency hashes and added husky package
apps/ledger-live-mobile/ios/Podfile.lock Updated pod dependency paths to match new pnpm hashes
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cunhabruno cunhabruno force-pushed the QAA-665-llm-add-type-check branch from 8ba1961 to 68155d5 Compare January 5, 2026 10:27
@cunhabruno cunhabruno requested a review from Copilot January 5, 2026 10:27
Copilot AI reviewed Jan 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

e2e/mobile/tsconfig.json
Comment on lines +5 to +7
"moduleResolution": "bundler",
"moduleDetection": "force",
"module": "preserve",
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combination of "moduleResolution": "bundler" with "module": "preserve" may not be optimal for a Node.js testing environment. The "bundler" module resolution is designed for bundlers like Webpack or Vite, not for Node.js. For a Node.js environment with modern TypeScript, consider using "moduleResolution": "node" or "nodenext" instead, along with an appropriate "module" setting like "commonjs" or "esnext".

Suggested change
"moduleResolution": "bundler",
"moduleDetection": "force",
"module": "preserve",
"moduleResolution": "nodenext",
"moduleDetection": "force",
"module": "esnext",

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@cunhabruno cunhabruno Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to set it as bundler and preserve, because we are importing modules from apps/ledger-live-mobile, typecheck would fail otherwise. Any other suggestion?

@cunhabruno cunhabruno force-pushed the QAA-948-llm-adapt-test-for-undefined-account-address branch 2 times, most recently from 01a63e0 to eab3b8d Compare January 5, 2026 11:33
@cunhabruno cunhabruno force-pushed the QAA-665-llm-add-type-check branch from 68155d5 to f72da60 Compare January 5, 2026 11:58
@live-github-bot live-github-bot bot added the common Has changes in live-common label Jan 5, 2026
Copilot AI review requested due to automatic review settings January 5, 2026 12:26
@cunhabruno cunhabruno force-pushed the QAA-665-llm-add-type-check branch from f72da60 to 883962b Compare January 5, 2026 12:26
@live-github-bot live-github-bot bot removed the common Has changes in live-common label Jan 5, 2026
Copilot AI reviewed Jan 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@live-github-bot
Copy link
Contributor

live-github-bot bot commented Jan 5, 2026

Mobile Bundle Checks

Comparing 8ea59e5 against 81d2efe.

🚀 main.ios.jsbundle bundle size decreased (54.9mb -> 54.8mb). Thanks ❤️
🚀 main.android.jsbundle bundle size decreased (55mb -> 54.9mb). Thanks ❤️

@cunhabruno cunhabruno marked this pull request as draft January 5, 2026 12:40
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Base automatically changed from QAA-948-llm-adapt-test-for-undefined-account-address to develop January 6, 2026 09:12
@cunhabruno cunhabruno closed this Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@cunhabruno cunhabruno

Labels

mobile Has changes in LLM

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cunhabruno