feat: add Telegram bot integration for food and exercise logging#1086
feat: add Telegram bot integration for food and exercise logging#1086serjsv87 wants to merge 5 commits intoCodeWithCJ:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a Telegram bot integration for SparkyFitness, enabling users to log food, exercise, water intake, and body measurements through AI-powered chat interactions. The implementation includes new services for bot management and intent execution, refactors core chat and logging modules to TypeScript, and adds necessary API endpoints and documentation. Feedback focuses on enhancing code quality and maintainability, specifically by replacing magic numbers with named constants, using standard TypeScript imports instead of require, and avoiding type suppressions like @ts-ignore. Additionally, it is recommended to remove pnpm-lock.yaml from the .gitignore file to ensure reproducible builds across different environments.
|
|
||
| # TypeScript | ||
| *.tsbuildinfo | ||
| pnpm-lock.yaml |
There was a problem hiding this comment.
Committing pnpm-lock.yaml is highly recommended to ensure that all developers and CI/CD environments use the exact same dependency versions, leading to reproducible builds. Ignoring it can cause "it works on my machine" issues when different versions of transitive dependencies are installed. Please consider removing pnpm-lock.yaml from this file.
| //@ts-ignore | ||
| code: arg.code, | ||
| //@ts-ignore | ||
| status: arg.status || arg.response?.status, | ||
| //@ts-ignore | ||
| responseData: arg.response?.data ? truncateStrings(arg.response.data, 500) : undefined |
There was a problem hiding this comment.
Using @ts-ignore should be avoided as it suppresses type-checking errors that might be valid. A better approach to handle properties on error objects that are not part of the standard Error type is to check for their existence or use type assertions.
code: (arg as any).code,
status: (arg as any).status || (arg as any).response?.status,
responseData: (arg as any).response?.data ? truncateStrings((arg as any).response.data, 500) : undefined| userId, | ||
| name | ||
| ); | ||
| } catch (e) { |
There was a problem hiding this comment.
The error object e in this catch block is implicitly typed as any. It's a good practice in TypeScript to explicitly type it as any or unknown to be clear about its type. This applies to other catch blocks in this file as well (e.g., lines 123, 169, 304).
| } catch (e) { | |
| } catch (e: any) { |
| // upsertWaterIntake takes change_drinks (drinks count), not ml directly. | ||
| // We pass ml as "drinks" but with no container — service will use default (250ml/drink). | ||
| // So we convert ml → drinks using default 250ml/drink. | ||
| const drinks = totalMl / 250; |
There was a problem hiding this comment.
The number 250 is used here to convert milliliters to a number of drinks. This appears to be a "magic number". It would be better to define this as a named constant to improve readability and make it easier to change if the default drink size is ever updated.
const DEFAULT_DRINK_SIZE_ML = 250;
const drinks = totalMl / DEFAULT_DRINK_SIZE_ML;| calories_per_hour: estimateCaloriesPerHour(name), | ||
| is_public: false, | ||
| source: 'telegram', | ||
| category: 'Cardio', |
There was a problem hiding this comment.
The exercise category is hardcoded as 'Cardio'. This might not be accurate for all types of exercises that could be logged, such as strength training. Consider making this more dynamic, perhaps by inferring it from the exercise name similar to how estimateCaloriesPerHour works, or by using a more generic default like 'General'. A simple improvement could be to check for keywords.
| import * as measurementRepository from '../../models/measurementRepository'; | ||
| import { executeIntent } from './intentExecutor'; | ||
| import axios from 'axios'; | ||
| const bmrService = require('../../services/bmrService'); |
There was a problem hiding this comment.
This file uses require to import bmrService. In a TypeScript file, it's better to use import statements for consistency and to leverage static analysis and type checking. This applies to other require calls in this file as well (e.g., for timezoneLoader on line 16).
| const bmrService = require('../../services/bmrService'); | |
| import bmrService from '../../services/bmrService'; |
| }; | ||
| }) | ||
| .filter((content) => content.parts.length > 0), | ||
| systemInstruction: undefined as any |
There was a problem hiding this comment.
The use of undefined as any is a code smell. If the type for systemInstruction doesn't allow undefined, it's better to omit the property entirely if it's not needed, rather than casting undefined to any. If the property can be omitted, conditional property assignment is cleaner.
| systemInstruction: undefined as any | |
| systemInstruction: undefined, |
|
Any reason why you still removed the checklist? I and all other maintainers still fill so its easier to understand the those who submit PR have done the pre-requisites to-do list properly. PR desc doesn't have any info about Garmin , but the Garmin related file still undergone some changes. Is that from your other PR that merged in here? Doc should be part of docs/content not the server\doc. This way they will be part of https://codewithcj.github.io/SparkyFitness/ |
d67f6eb to
f8f7682
Compare
f8f7682 to
dae10b1
Compare
|
I have restored the checklist and fixed the formatting issues. I have also removed the unrelated Garmin refactor from this PR to keep it focused only on the Telegram integration. The minor call to garminService remains to support the manual 'Sync Garmin' button in the bot. Lastly, I've moved the setup documentation to docs/content/8.developer/12.telegram-bot-setup.md as requested. |
…ly food logs into system context.
|
pnpm-lock.yaml shouldn't be in gitingore. |
| try { | ||
| const mfpProviders = | ||
| await externalProviderRepository.getProvidersByType('myfitnesspal'); | ||
| const today = new Date().toISOString().split('T')[0]; |
There was a problem hiding this comment.
toISOString() converts to UTC which may be tomorrow depending on the server's configured time zone (may be different for non docker deployments)
use the localDateToDay function in the @workspace/shared to get the current day
|
|
||
| # TypeScript | ||
| *.tsbuildinfo | ||
| pnpm-lock.yaml |
There was a problem hiding this comment.
pnpm-lock.yaml should be commited
| export * from "./constants/calorieConstants.ts"; | ||
| export * from "./utils/timezone.ts"; | ||
| export * from "./utils/calorieCalculations.ts"; | ||
| export * from "./schemas/api/AiServiceSettings.api.zod"; |
There was a problem hiding this comment.
This will cause module resolution errors on the server. The .ts extensions were recently added. Why remove them?
| @@ -0,0 +1,278 @@ | |||
| import { log } from '../../config/logging'; | |||
| import axios from 'axios'; | |||
| const externalProviderRepository = require('../../models/externalProviderRepository'); | |||
There was a problem hiding this comment.
Try you best to not mix import and require
| app.use('/api/integrations/polar', polarRoutes); | ||
| app.use('/api/integrations/strava', stravaRoutes); | ||
| app.use('/api/integrations/hevy', hevyRoutes); | ||
| app.use('/api/integrations/myfitnesspal', mfpRoutes); |
There was a problem hiding this comment.
Left over from split PR I believe..?
| }); | ||
| }; | ||
|
|
||
| // MyFitnessPal sync |
|
The telegram integration seems really cool, thanks for the work on it. I left some comments on the code. I'd also recommend splitting up There do seem to be some files left over from when you split the PR. Do a search for MFP and MyFitnessPal and you'll see a couple. Also the garmin file included in this PR is the whole thing in typescript, not just a call. |

Introduces a Telegram bot that allows users to log food, exercises, measurements, and water directly via chat using AI-powered intent recognition.
Added telegramBotService and intentExecutor.
Added chatRepository and chatService (migrated to TS).
Added webhook and polling support.
Added /api/telegram routes for account linking.
Added TELEGRAM_SETUP.md documentation.
Included only English translations.
✅ Checklist