chore: organize code in modules, absolute imports, better exports #47
Conversation
|
@wtfsayo after this is merged i can take a look at: |
|
|
|
I see some imports use |
|
we can use absolute import everywhere @HashWarlock, where you see the imports with |
|
why inconsistent with @/... path and ../ |
aight let me fix those, also another thing I've seen is having a convention for naming files. Example:
For convenience I would say define a standard and adapt the code. Maybe just a separate doc for the whole elizaOS organization, which applies to all repos @wtfsayo @lalalune |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request involves a comprehensive restructuring of the project's module organization, focusing on improving code structure, import paths, and export mechanisms. The changes primarily involve moving files to new directories, updating import statements to use absolute paths with the Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Config as Rollup/TypeScript Config
participant Modules as Project Modules
Dev->>Config: Update import paths
Config->>Modules: Resolve using @/ alias
Modules-->>Config: Return correct module
Config-->>Dev: Successful module resolution
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/features/spaces/plugins/MonitorAudioPlugin.ts (1)
Line range hint
24-54: Add error handling for missing ffplay dependency.The constructor spawns ffplay without first verifying its availability. This could lead to runtime errors if ffplay is not installed on the system.
Consider adding a check before spawning:
constructor(private readonly sampleRate = 48000, debug = false) { this.logger = new Logger(debug); + + // Verify ffplay is available + try { + spawn('ffplay', ['-version']).kill(); + } catch (error) { + throw new Error('ffplay is not installed. Please install ffmpeg/ffplay to use MonitorAudioPlugin.'); + } // Spawn ffplay to read raw PCM (s16le) on stdinAlso, consider adding input validation:
constructor(private readonly sampleRate = 48000, debug = false) { + if (sampleRate <= 0 || !Number.isInteger(sampleRate)) { + throw new Error('Sample rate must be a positive integer'); + } this.logger = new Logger(debug);src/features/relationships/relationships.ts (1)
Line range hint
82-124: Reduce code duplication in timeline functionsThe
getFollowingTimelineandgetFollowersTimelinefunctions share significant code. Consider extracting the common logic into a base function.+type TimelineType = 'following' | 'followers'; + +interface TimelineConfig { + endpoint: string; + errorMessage: string; +} + +const TIMELINE_CONFIGS: Record<TimelineType, TimelineConfig> = { + following: { + endpoint: 'iSicc7LrzWGBgDPL0tM_TQ/Following', + errorMessage: 'Scraper is not logged-in for profile following.', + }, + followers: { + endpoint: 'rRXFSG5vR6drKr5M37YOTw/Followers', + errorMessage: 'Scraper is not logged-in for profile followers.', + }, +}; + +async function getRelationshipTimeline( + type: TimelineType, + userId: string, + maxItems: number, + auth: TwitterAuth, + cursor?: string, +): Promise<RelationshipTimeline> { + if (!auth.isLoggedIn()) { + throw new Error(TIMELINE_CONFIGS[type].errorMessage); + } + + maxItems = validateMaxItems(maxItems); + + const variables: Record<string, any> = { + userId, + count: maxItems, + includePromotedContent: false, + }; + + if (cursor != null && cursor != '') { + variables['cursor'] = cursor; + } + + const features = addApiFeatures(TIMELINE_API_FEATURES); + const params = new URLSearchParams(); + params.set('features', stringify(features) ?? ''); + params.set('variables', stringify(variables) ?? ''); + + const res = await requestApi<RelationshipTimeline>( + `https://twitter.com/i/api/graphql/${TIMELINE_CONFIGS[type].endpoint}?${params.toString()}`, + auth, + ); + + if (!res.success) { + throw res.err; + } + + return res.value; +} + +async function getFollowingTimeline( + userId: string, + maxItems: number, + auth: TwitterAuth, + cursor?: string, +): Promise<RelationshipTimeline> { + return getRelationshipTimeline('following', userId, maxItems, auth, cursor); +} + +async function getFollowersTimeline( + userId: string, + maxItems: number, + auth: TwitterAuth, + cursor?: string, +): Promise<RelationshipTimeline> { + return getRelationshipTimeline('followers', userId, maxItems, auth, cursor); +}Also applies to: 139-181
🧰 Tools
🪛 eslint
[error] 10-12: Delete
⏎⏎(prettier/prettier)
🧹 Nitpick comments (19)
src/features/spaces/plugins/MonitorAudioPlugin.ts (1)
Line range hint
1-15: Enhance documentation with dependency requirements.The documentation is clear, but consider adding information about external dependencies (ffplay) and system requirements. Also, the comment escape in the usage example could be improved.
* Usage: - * const plugin = new MonitorAudioPlugin(48000, /* debug= *\/ true); + * const plugin = new MonitorAudioPlugin(48000, /* debug= */ true); * space.use(plugin);src/features/relationships/relationships.ts (3)
Line range hint
89-90: Extract maxItems validation to a shared utilityThe maxItems validation is duplicated in both timeline functions. Consider extracting it to a shared utility function to maintain DRY principles.
+const MAX_TIMELINE_ITEMS = 50; + +function validateMaxItems(items: number): number { + return Math.min(items, MAX_TIMELINE_ITEMS); +} + async function getFollowingTimeline( userId: string, maxItems: number, auth: TwitterAuth, cursor?: string, ): Promise<RelationshipTimeline> { if (!auth.isLoggedIn()) { throw new Error('Scraper is not logged-in for profile following.'); } - if (maxItems > 50) { - maxItems = 50; - } + maxItems = validateMaxItems(maxItems);Also applies to: 146-147
🧰 Tools
🪛 eslint
[error] 10-12: Delete
⏎⏎(prettier/prettier)
Line range hint
93-101: Extract common API features configurationThe API features configuration is duplicated across timeline functions. Consider extracting it to a shared constant.
+const TIMELINE_API_FEATURES = { + responsive_web_twitter_article_tweet_consumption_enabled: false, + tweet_with_visibility_results_prefer_gql_limited_actions_policy_enabled: true, + longform_notetweets_inline_media_enabled: true, + responsive_web_media_download_video_enabled: false, +}; + async function getFollowingTimeline( // ... parameters ... ): Promise<RelationshipTimeline> { // ... auth check and maxItems validation ... - const features = addApiFeatures({ - responsive_web_twitter_article_tweet_consumption_enabled: false, - tweet_with_visibility_results_prefer_gql_limited_actions_policy_enabled: true, - longform_notetweets_inline_media_enabled: true, - responsive_web_media_download_video_enabled: false, - }); + const features = addApiFeatures(TIMELINE_API_FEATURES);Also applies to: 150-158
🧰 Tools
🪛 eslint
[error] 10-12: Delete
⏎⏎(prettier/prettier)
Line range hint
183-248: Enhance error handling in followUser functionThe error handling in the
followUserfunction could be improved:
- Consider adding specific error types for different failure scenarios
- Add retry logic for transient failures
- Add rate limiting handling
+interface TwitterError { + errors?: Array<{ code: number; message: string }>; +} + +class FollowError extends Error { + constructor( + message: string, + public readonly code?: number, + public readonly response?: Response + ) { + super(message); + } +} + export async function followUser( username: string, auth: TwitterAuth, + retries = 3 ): Promise<Response> { // Check if the user is logged in if (!(await auth.isLoggedIn())) { - throw new Error('Must be logged in to follow users'); + throw new FollowError('Authentication required to follow users', 401); } // Get user ID from username const userIdResult = await getUserIdByScreenName(username, auth); if (!userIdResult.success) { - throw new Error(`Failed to get user ID: ${userIdResult.err.message}`); + throw new FollowError( + `Failed to resolve user ID for ${username}`, + 404, + userIdResult.err + ); } + let lastError: Error | null = null; + for (let attempt = 1; attempt <= retries; attempt++) { + try { + return await attemptFollow(username, userIdResult.value, auth); + } catch (error) { + lastError = error; + if (error instanceof FollowError && error.code === 429) { + // Rate limited - wait before retry + await new Promise(resolve => setTimeout(resolve, attempt * 1000)); + continue; + } + throw error; + } + } + throw lastError || new FollowError('Max retries exceeded'); +} + +async function attemptFollow( + username: string, + userId: string, + auth: TwitterAuth +): Promise<Response> { // ... rest of the implementation ... }🧰 Tools
🪛 eslint
[error] 10-12: Delete
⏎⏎(prettier/prettier)
src/scraper/scraper.ts (2)
105-108: Update import path forcreateGrokConversationandgrokChatThe functions
createGrokConversationandgrokChatare imported from'../grok', which is a relative path. To maintain consistency with the use of absolute imports, consider updating this import statement to use the@/alias if applicable.Apply this diff to update the import path:
-import { - createGrokConversation, - grokChat, - GrokChatOptions, - GrokChatResponse, -} from '../grok'; +import { + createGrokConversation, + grokChat, + GrokChatOptions, + GrokChatResponse, +} from '@/grok';
Line range hint
362-367: Deprecation warnings forwithCookieandwithXCsrfTokenmethodsThe methods
withCookieandwithXCsrfTokenare marked as deprecated and include console warnings. Consider removing these methods in a future release to clean up the codebase and prevent their use.Apply this diff to deprecate the methods properly:
/** * Sets the optional cookie to be used in requests. * @param _cookie The cookie to be used in requests. - * @deprecated This function no longer represents any part of Twitter's auth flow. + * @deprecated Use Scraper#setCookies instead. * @returns This scraper instance. */And consider adding the
@deprecatedannotation in TypeScript to provide better tooling support.src/features/spaces/core/ChatClient.ts (3)
2-2: Maintain consistent import orderingThe import statements have been reordered. To improve code readability, group similar imports together and maintain a consistent ordering, such as:
- Node.js built-in modules
- Third-party modules
- Local modules
Apply this diff to reorder the imports:
import { EventEmitter } from 'events'; import { Logger } from '../logger'; import type { OccupancyUpdate, SpeakerRequest } from '../types'; +import WebSocket from 'ws';Alternatively, if
WebSocketis a third-party module, it should be grouped with other third-party imports.
81-99: Optional chaining unnecessary when WebSocket instance is guaranteedIn the
setupHandlersmethod, using optional chaining?.onthis.wsmight be unnecessary since a check is performed earlier to ensurethis.wsis available.Consider replacing
this.ws?.onwiththis.ws.onto reflect thatthis.wsis indeed defined at this point.Apply this diff:
- this.ws?.on('open', () => { + this.ws.on('open', () => {Repeat this change for all subsequent event handler registrations within the method.
Line range hint
185-187: Improve error handling insafeJsonfunctionCurrently, the
safeJsonfunction silently fails and returnsnullon invalid JSON. Consider logging the error or providing more context to aid in debugging.Apply this diff to log the error:
function safeJson(text: string): any { try { return JSON.parse(text); } catch (error) { + console.error('[ChatClient] Failed to parse JSON:', error); return null; } }Alternatively, use the
Loggerinstance for consistent logging:function safeJson(text: string): any { try { return JSON.parse(text); } catch (error) { + this.logger.error('[ChatClient] Failed to parse JSON:', error); return null; } }Note: Ensure
safeJsonhas access to theloggerinstance if using it.src/timeline/types/timeline-v2.ts (1)
1-5: Fix quote style to maintain consistency.The import statements use double quotes, but the project style guide (as indicated by ESLint) prefers single quotes.
Apply this diff to fix the quote style:
-import { isFieldDefined } from "@/core/utils"; -import { LegacyUserRaw } from "@/features/profiles"; -import { Tweet } from "@/features/tweets"; -import { parseMediaGroups, reconstructTweetHtml } from "../components"; -import { LegacyTweetRaw, ParseTweetResult, QueryTweetsResponse, SearchResultRaw, TimelineResultRaw } from "./timeline-v1"; +import { isFieldDefined } from '@/core/utils'; +import { LegacyUserRaw } from '@/features/profiles'; +import { Tweet } from '@/features/tweets'; +import { parseMediaGroups, reconstructTweetHtml } from '../components'; +import { + LegacyTweetRaw, + ParseTweetResult, + QueryTweetsResponse, + SearchResultRaw, + TimelineResultRaw, +} from './timeline-v1';🧰 Tools
🪛 eslint
[error] 1-1: Replace
"@/core/utils"with'@/core/utils'(prettier/prettier)
[error] 2-2: Replace
"@/features/profiles"with'@/features/profiles'(prettier/prettier)
[error] 3-3: Replace
"@/features/tweets"with'@/features/tweets'(prettier/prettier)
[error] 4-4: Replace
"../components"with'../components'(prettier/prettier)
src/cli/command.ts (1)
Line range hint
1-16: Consider using a dedicated platform detection module.The platform detection logic could be moved to a separate module to improve maintainability and reusability.
Consider creating a new file
src/core/platform.ts:// src/core/platform.ts export const PLATFORM_NODE = typeof process !== 'undefined' && ( // Node.js check (process.versions?.node != null) || // Bun check (process.versions?.bun != null) ); export const PLATFORM_NODE_JEST = false;Then update the imports:
-// Declare the types for our custom global properties -declare global { - var PLATFORM_NODE: boolean; - var PLATFORM_NODE_JEST: boolean; -} - -// Define platform constants before imports -globalThis.PLATFORM_NODE = typeof process !== 'undefined' && ( - // Node.js check - (process.versions?.node != null) || - // Bun check - (process.versions?.bun != null) -); -globalThis.PLATFORM_NODE_JEST = false; +import { PLATFORM_NODE, PLATFORM_NODE_JEST } from '@/core/platform';tsconfig.json (1)
Line range hint
3-24: LGTM! Path alias configuration is well structured.The configuration properly sets up absolute imports with the
@/*alias, which will help standardize import paths across the codebase. ThebaseUrlandpathssettings are correctly configured for TypeScript path resolution.Consider adding a trailing comma after the
pathsobject for consistency with the rest of the file:"paths": { "@/*": ["src/*"] - } + },src/timeline/components/timeline-tweet-util.ts (1)
1-3: Update quotes to use single quotes for consistency.The import statements are correctly using the new path aliases, which improves code organization. However, the quote style should be standardized.
-import { isFieldDefined, NonNullableField } from "@/core/utils"; -import { Photo, Video } from "@/features/tweets"; -import { LegacyTweetRaw, TimelineMediaExtendedRaw } from "../types"; +import { isFieldDefined, NonNullableField } from '@/core/utils'; +import { Photo, Video } from '@/features/tweets'; +import { LegacyTweetRaw, TimelineMediaExtendedRaw } from '../types';🧰 Tools
🪛 eslint
[error] 1-1: Replace
"@/core/utils"with'@/core/utils'(prettier/prettier)
[error] 2-2: Replace
"@/features/tweets"with'@/features/tweets'(prettier/prettier)
src/features/tweets/tweets.test.ts (2)
Line range hint
1-11: Consider enhancing security for API credentials in tests.While the test properly checks for environment variables before running API tests, it's recommended to:
- Use a mock API service for tests to avoid requiring real credentials
- Document the required environment variables in the README
- Provide example values for the environment variables
🧰 Tools
🪛 eslint
[error] 7-8: Delete
⏎(prettier/prettier)
Line range hint
32-38: Add error handling for API rate limits.The test for getting tweets from a list should handle potential API rate limits.
const res: QueryTweetsResponse = await scraper.fetchListTweets( '1736495155002106192', maxTweets, cursor, ); + if (res.tweets.length === 0) { + console.warn('No tweets returned. Possible rate limit.'); + }🧰 Tools
🪛 eslint
[error] 7-8: Delete
⏎(prettier/prettier)
src/features/tweets/tweets.ts (1)
481-481: Consider adding rate limiting mechanism.The API interactions could benefit from a rate limiting mechanism to prevent hitting Twitter's API limits.
Consider implementing a rate limiter utility:
// src/core/utils/rate-limiter.ts export class RateLimiter { private timestamps: number[] = []; constructor(private windowMs: number, private maxRequests: number) {} async throttle(): Promise<void> { const now = Date.now(); this.timestamps = this.timestamps.filter(t => now - t < this.windowMs); if (this.timestamps.length >= this.maxRequests) { const oldestTimestamp = this.timestamps[0]; const waitTime = this.windowMs - (now - oldestTimestamp); await new Promise(resolve => setTimeout(resolve, waitTime)); } this.timestamps.push(now); } }src/timeline/components/timeline-search.ts (1)
1-5: Import paths have been properly reorganized.The changes align well with the modular architecture, separating core utilities and feature-specific imports. However, consider using consistent import quote styles (single vs double quotes) across the codebase.
-import { isFieldDefined } from "@/core/utils"; -import { LegacyUserRaw, parseProfile, Profile } from "@/features/profiles"; -import { PlaceRaw, Tweet } from "@/features/tweets"; -import { parseMediaGroups, reconstructTweetHtml } from "../components"; +import { isFieldDefined } from '@/core/utils'; +import { LegacyUserRaw, parseProfile, Profile } from '@/features/profiles'; +import { PlaceRaw, Tweet } from '@/features/tweets'; +import { parseMediaGroups, reconstructTweetHtml } from '../components';🧰 Tools
🪛 eslint
[error] 1-2: Delete
⏎(prettier/prettier)
[error] 4-4: Replace
·QueryProfilesResponse,·QueryTweetsResponse·with⏎··QueryProfilesResponse,⏎··QueryTweetsResponse,⏎(prettier/prettier)
src/grok.ts (1)
1-2: Core functionality properly organized into dedicated modules.The changes correctly move core functionality into a dedicated core directory. However, consider using consistent import quote styles (single vs double quotes) across the codebase.
-import { requestApi } from "./core/api"; -import { TwitterAuth } from "./core/auth"; +import { requestApi } from './core/api'; +import { TwitterAuth } from './core/auth';🧰 Tools
🪛 eslint
[error] 1-1: Replace
"./core/api"with'./core/api'(prettier/prettier)
src/timeline/types/timeline-v1.ts (1)
1-5: Import paths properly organized into feature modules.The changes successfully separate core utilities and feature-specific types into their respective modules. However, consider using consistent import quote styles (single vs double quotes) across the codebase.
-import { isFieldDefined } from "@/core/utils"; -import { LegacyUserRaw, parseProfile, Profile } from "@/features/profiles"; -import { PlaceRaw, Tweet } from "@/features/tweets"; -import { parseMediaGroups, reconstructTweetHtml } from "../components"; +import { isFieldDefined } from '@/core/utils'; +import { LegacyUserRaw, parseProfile, Profile } from '@/features/profiles'; +import { PlaceRaw, Tweet } from '@/features/tweets'; +import { parseMediaGroups, reconstructTweetHtml } from '../components';🧰 Tools
🪛 eslint
[error] 1-1: Replace
"@/core/utils"with'@/core/utils'(prettier/prettier)
[error] 2-2: Replace
"@/features/profiles"with'@/features/profiles'(prettier/prettier)
[error] 3-3: Replace
"@/features/tweets"with'@/features/tweets'(prettier/prettier)
[error] 4-5: Replace
"../components";⏎with'../components';(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
rollup.config.mjs(3 hunks)src/_module.ts(0 hunks)src/cli/command.ts(1 hunks)src/cli/index.ts(1 hunks)src/core/api/api.ts(1 hunks)src/core/api/index.ts(1 hunks)src/core/auth/auth-user.ts(1 hunks)src/core/auth/auth.test.ts(1 hunks)src/core/auth/auth.ts(1 hunks)src/core/auth/index.ts(1 hunks)src/core/types/index.ts(1 hunks)src/core/utils/index.ts(1 hunks)src/core/utils/test-utils.ts(1 hunks)src/features/index.ts(1 hunks)src/features/messages/index.ts(1 hunks)src/features/messages/messages.test.ts(1 hunks)src/features/messages/messages.ts(1 hunks)src/features/profiles/index.ts(1 hunks)src/features/profiles/profile.test.ts(1 hunks)src/features/profiles/profile.ts(1 hunks)src/features/relationships/index.ts(1 hunks)src/features/relationships/relationships.test.ts(1 hunks)src/features/relationships/relationships.ts(1 hunks)src/features/search/index.ts(1 hunks)src/features/search/search.test.ts(1 hunks)src/features/search/search.ts(1 hunks)src/features/spaces/core/ChatClient.ts(2 hunks)src/features/spaces/core/JanusAudio.ts(1 hunks)src/features/spaces/core/JanusClient.ts(3 hunks)src/features/spaces/core/Space.ts(1 hunks)src/features/spaces/core/SpaceParticipant.ts(1 hunks)src/features/spaces/core/index.ts(1 hunks)src/features/spaces/index.ts(1 hunks)src/features/spaces/logger.ts(0 hunks)src/features/spaces/plugins/HlsRecordPlugin.ts(9 hunks)src/features/spaces/plugins/IdleMonitorPlugin.ts(3 hunks)src/features/spaces/plugins/MonitorAudioPlugin.ts(4 hunks)src/features/spaces/plugins/index.ts(1 hunks)src/features/spaces/test.ts(1 hunks)src/features/spaces/testParticipant.ts(1 hunks)src/features/spaces/types.ts(0 hunks)src/features/spaces/utils.ts(0 hunks)src/features/trends/index.ts(1 hunks)src/features/trends/trends.test.ts(1 hunks)src/features/trends/trends.ts(1 hunks)src/features/tweets/index.ts(1 hunks)src/features/tweets/tweets.test.ts(1 hunks)src/features/tweets/tweets.ts(3 hunks)src/grok.ts(1 hunks)src/index.ts(1 hunks)src/scraper/index.ts(1 hunks)src/scraper/scraper.test.ts(1 hunks)src/scraper/scraper.ts(2 hunks)src/spaces.ts(2 hunks)src/timeline/components/index.ts(1 hunks)src/timeline/components/timeline-async.ts(1 hunks)src/timeline/components/timeline-following.ts(1 hunks)src/timeline/components/timeline-home.ts(1 hunks)src/timeline/components/timeline-list.ts(1 hunks)src/timeline/components/timeline-relationship.ts(1 hunks)src/timeline/components/timeline-search.ts(1 hunks)src/timeline/components/timeline-tweet-util.ts(1 hunks)src/timeline/index.ts(1 hunks)src/timeline/types/index.ts(1 hunks)src/timeline/types/timeline-v1.ts(1 hunks)src/timeline/types/timeline-v2.ts(1 hunks)tsconfig.json(2 hunks)
💤 Files with no reviewable changes (4)
- src/features/spaces/logger.ts
- src/features/spaces/types.ts
- src/features/spaces/utils.ts
- src/_module.ts
✅ Files skipped from review due to trivial changes (47)
- src/features/messages/index.ts
- src/core/types/index.ts
- src/features/profiles/index.ts
- src/features/relationships/index.ts
- src/features/tweets/index.ts
- src/features/search/index.ts
- src/cli/index.ts
- src/core/auth/index.ts
- src/core/utils/index.ts
- src/features/spaces/plugins/IdleMonitorPlugin.ts
- src/features/spaces/core/index.ts
- src/features/spaces/core/JanusAudio.ts
- src/features/spaces/plugins/HlsRecordPlugin.ts
- src/features/spaces/index.ts
- src/features/trends/trends.test.ts
- src/features/spaces/core/SpaceParticipant.ts
- src/features/messages/messages.ts
- src/features/spaces/plugins/index.ts
- src/features/relationships/relationships.test.ts
- src/scraper/scraper.test.ts
- src/features/spaces/core/Space.ts
- src/core/api/index.ts
- src/features/spaces/testParticipant.ts
- src/core/auth/auth.test.ts
- src/features/profiles/profile.test.ts
- src/features/search/search.test.ts
- rollup.config.mjs
- src/features/messages/messages.test.ts
- src/features/index.ts
- src/features/trends/index.ts
- src/scraper/index.ts
- src/features/trends/trends.ts
- src/timeline/components/timeline-list.ts
- src/core/utils/test-utils.ts
- src/spaces.ts
- src/core/auth/auth.ts
- src/timeline/types/index.ts
- src/timeline/index.ts
- src/timeline/components/timeline-following.ts
- src/core/api/api.ts
- src/timeline/components/timeline-home.ts
- src/timeline/components/timeline-relationship.ts
- src/timeline/components/timeline-async.ts
- src/features/profiles/profile.ts
- src/features/search/search.ts
- src/features/spaces/core/JanusClient.ts
- src/timeline/components/index.ts
🧰 Additional context used
🪛 eslint
src/timeline/types/timeline-v2.ts
[error] 1-1: Replace "@/core/utils" with '@/core/utils'
(prettier/prettier)
[error] 2-2: Replace "@/features/profiles" with '@/features/profiles'
(prettier/prettier)
[error] 3-3: Replace "@/features/tweets" with '@/features/tweets'
(prettier/prettier)
[error] 4-4: Replace "../components" with '../components'
(prettier/prettier)
[error] 5-6: Replace ·LegacyTweetRaw,·ParseTweetResult,·QueryTweetsResponse,·SearchResultRaw,·TimelineResultRaw·}·from·"./timeline-v1";⏎ with ⏎··LegacyTweetRaw,⏎··ParseTweetResult,⏎··QueryTweetsResponse,⏎··SearchResultRaw,⏎··TimelineResultRaw,⏎}·from·'./timeline-v1';
(prettier/prettier)
src/grok.ts
[error] 1-1: Replace "./core/api" with './core/api'
(prettier/prettier)
[error] 2-3: Replace "./core/auth";⏎ with './core/auth';
(prettier/prettier)
src/timeline/components/timeline-search.ts
[error] 1-2: Delete ⏎
(prettier/prettier)
[error] 4-4: Replace ·QueryProfilesResponse,·QueryTweetsResponse· with ⏎··QueryProfilesResponse,⏎··QueryTweetsResponse,⏎
(prettier/prettier)
src/timeline/components/timeline-tweet-util.ts
[error] 1-1: Replace "@/core/utils" with '@/core/utils'
(prettier/prettier)
[error] 2-2: Replace "@/features/tweets" with '@/features/tweets'
(prettier/prettier)
[error] 3-4: Replace "../types";⏎ with '../types';
(prettier/prettier)
src/timeline/types/timeline-v1.ts
[error] 1-1: Replace "@/core/utils" with '@/core/utils'
(prettier/prettier)
[error] 2-2: Replace "@/features/profiles" with '@/features/profiles'
(prettier/prettier)
[error] 3-3: Replace "@/features/tweets" with '@/features/tweets'
(prettier/prettier)
[error] 4-5: Replace "../components";⏎ with '../components';
(prettier/prettier)
🔇 Additional comments (9)
src/scraper/scraper.ts (4)
6-12: Consistent usage of absolute import paths throughout the codebaseThe transition from relative imports to absolute imports using the
@/alias enhances code readability and maintainability. This change aligns with the project's objective to improve the modular structure of the codebase.Also applies to: 14-19, 21-25, 29-31, 33-38, 45-46, 48-72
74-81: Ensure that the@/spacesmodule exports required functionsThe imports from
'@/spaces'for functions likefetchAudioSpaceById,fetchAuthenticatePeriscope, etc., indicate that these functions have been moved to thespacesmodule. Verify that the@/spacesmodule correctly exports these functions to prevent any runtime import errors.
81-90: Check for compatibility with timeline-related importsThe imports related to timelines, such as
fetchFollowingTimeline,fetchHomeTimeline, and others, have been updated to use absolute paths from'@/timeline'. Ensure that these functions are properly defined and exported in thetimelinemodule, and that any dependencies are correctly resolved.
Line range hint
143-146: Verify the hardcoded URLs and constantsThe URLs such as
twUrlandUserTweetsUrlare hardcoded. Ensure that these endpoints are still valid and have not changed, especially given Twitter's frequent API updates.Run the following script to check the validity of the URLs:
✅ Verification successful
The hardcoded URLs and GraphQL endpoints are valid and current
The URLs follow Twitter's current API structure and are consistently used across multiple files in the codebase. The 403 responses are expected as these endpoints require authentication, not because they're invalid. The GraphQL operation ID is actively used in the production code, indicating it's a current endpoint.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the hardcoded URLs are still valid. # Check the main Twitter URL curl -I https://twitter.com -o /dev/null -w '%{http_code}\n' -s # Check the UserTweets API endpoint curl -I 'https://twitter.com/i/api/graphql/E3opETHurmVJflFsUBVuUQ/UserTweets' -o /dev/null -w '%{http_code}\n' -sLength of output: 187
Script:
#!/bin/bash # Search for the GraphQL operation ID and URL patterns echo "=== Searching for GraphQL operation ID ===" rg "E3opETHurmVJflFsUBVuUQ" -l echo -e "\n=== Searching for Twitter API URL patterns ===" rg "twitter\.com/i/api/graphql" -l echo -e "\n=== Looking for URL configurations ===" fd -e json -e ts -e js -e yaml -e yml . | xargs rg "twUrl|UserTweetsUrl|baseUrl" echo -e "\n=== Checking recent changes to scraper.ts ===" git log -n 5 --oneline src/scraper/scraper.tsLength of output: 1337
src/features/spaces/test.ts (1)
1-1: LGTM! Import paths updated to match new project structure.The changes align with the PR objective of using absolute imports for better clarity and maintainability.
Also applies to: 4-5
src/core/auth/auth-user.ts (1)
1-1: LGTM! Import paths updated to maintain clear module boundaries.The changes appropriately use:
- Absolute paths (
@/) for feature imports- Relative paths for core functionality imports
Also applies to: 7-9
src/cli/command.ts (1)
17-22: LGTM! Import paths updated to match new project structure.The changes correctly use relative paths for importing from the new feature-based modules.
src/features/tweets/tweets.ts (1)
1-17: LGTM! Import organization is clean and follows the new structure.The imports are properly organized using the new path aliases, improving code maintainability.
src/timeline/types/timeline-v1.ts (1)
1-5: Verify consistent usage of path aliases across the codebase.Let's verify that all imports consistently use the new path aliases and identify any remaining relative imports that should be updated.
✅ Verification successful
Import paths follow consistent conventions
The codebase follows a clear pattern where '@/' aliases are used for cross-feature imports while relative paths are used for imports within the same feature. The imports in the reviewed file adhere to these conventions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent import paths # Find relative imports that could be using path aliases rg -g '*.ts' -g '*.tsx' "from '\.\./|from \"\.\." --no-heading # Find inconsistent usage of path aliases rg -g '*.ts' -g '*.tsx' "from ['\"]((@/[^'\"]+)|([^./@][^'\"]+))" --no-headingLength of output: 13711
🧰 Tools
🪛 eslint
[error] 1-1: Replace
"@/core/utils"with'@/core/utils'(prettier/prettier)
[error] 2-2: Replace
"@/features/profiles"with'@/features/profiles'(prettier/prettier)
[error] 3-3: Replace
"@/features/tweets"with'@/features/tweets'(prettier/prettier)
[error] 4-5: Replace
"../components";⏎with'../components';(prettier/prettier)
| onAudioData(data: AudioDataWithUser): void { | ||
| // Log debug info | ||
| this.logger.debug( | ||
| `[MonitorAudioPlugin] onAudioData => userId=${data.userId}, samples=${data.samples.length}, sampleRate=${data.sampleRate}`, | ||
| `[MonitorAudioPlugin] onAudioData => userId=${data.userId}, samples=${data.samples.length}, sampleRate=${data.sampleRate}`, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add sample rate validation and error handling.
The onAudioData method makes assumptions about sample rates and lacks error handling for buffer operations.
Consider adding these safety checks:
onAudioData(data: AudioDataWithUser): void {
+ if (data.sampleRate !== this.sampleRate) {
+ this.logger.error(
+ `[MonitorAudioPlugin] Sample rate mismatch: expected ${this.sampleRate}, got ${data.sampleRate}`
+ );
+ return;
+ }
this.logger.debug(
`[MonitorAudioPlugin] onAudioData => userId=${data.userId}, samples=${data.samples.length}, sampleRate=${data.sampleRate}`,
);
if (!this.ffplay?.stdin.writable) {
return;
}
const pcmBuffer = Buffer.from(data.samples.buffer);
- this.ffplay.stdin.write(pcmBuffer);
+ try {
+ const success = this.ffplay.stdin.write(pcmBuffer);
+ if (!success) {
+ this.logger.warn('[MonitorAudioPlugin] Buffer full, may need to implement backpressure handling');
+ }
+ } catch (error) {
+ this.logger.error('[MonitorAudioPlugin] Error writing to ffplay:', error);
+ }Committable suggestion skipped: line range outside the PR's diff.
| import { addApiFeatures, bearerToken, requestApi } from '@/core/api'; | ||
| import { TwitterAuth } from '@/core/auth'; | ||
| import { parseRelationshipTimeline, RelationshipTimeline } from '@/timeline'; | ||
| import { getUserTimeline } from '@/timeline/components/timeline-async'; | ||
| import { QueryProfilesResponse } from '@/timeline/types/timeline-v1'; | ||
| import { Headers } from 'headers-polyfill'; | ||
| import { TwitterAuth } from './auth'; | ||
| import { Profile, getUserIdByScreenName } from './profile'; | ||
| import { QueryProfilesResponse } from './timeline-v1'; | ||
| import { getUserTimeline } from './timeline-async'; | ||
| import { | ||
| RelationshipTimeline, | ||
| parseRelationshipTimeline, | ||
| } from './timeline-relationship'; | ||
| import stringify from 'json-stable-stringify'; | ||
| import { getUserIdByScreenName, Profile } from '../profiles'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Convert remaining relative import to absolute path
Per the PR objectives of standardizing imports to use absolute paths with the @/ prefix, the relative import from ../profiles should be updated to maintain consistency.
-import { getUserIdByScreenName, Profile } from '../profiles';
+import { getUserIdByScreenName, Profile } from '@/features/profiles';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { addApiFeatures, bearerToken, requestApi } from '@/core/api'; | |
| import { TwitterAuth } from '@/core/auth'; | |
| import { parseRelationshipTimeline, RelationshipTimeline } from '@/timeline'; | |
| import { getUserTimeline } from '@/timeline/components/timeline-async'; | |
| import { QueryProfilesResponse } from '@/timeline/types/timeline-v1'; | |
| import { Headers } from 'headers-polyfill'; | |
| import { TwitterAuth } from './auth'; | |
| import { Profile, getUserIdByScreenName } from './profile'; | |
| import { QueryProfilesResponse } from './timeline-v1'; | |
| import { getUserTimeline } from './timeline-async'; | |
| import { | |
| RelationshipTimeline, | |
| parseRelationshipTimeline, | |
| } from './timeline-relationship'; | |
| import stringify from 'json-stable-stringify'; | |
| import { getUserIdByScreenName, Profile } from '../profiles'; | |
| import { addApiFeatures, bearerToken, requestApi } from '@/core/api'; | |
| import { TwitterAuth } from '@/core/auth'; | |
| import { parseRelationshipTimeline, RelationshipTimeline } from '@/timeline'; | |
| import { getUserTimeline } from '@/timeline/components/timeline-async'; | |
| import { QueryProfilesResponse } from '@/timeline/types/timeline-v1'; | |
| import { Headers } from 'headers-polyfill'; | |
| import stringify from 'json-stable-stringify'; | |
| import { getUserIdByScreenName, Profile } from '@/features/profiles'; |
src/index.ts
Outdated
| export { Scraper } from './scraper'; | ||
|
|
||
| export * from '@/cli'; | ||
| // Not sure if this is needed | ||
| // export * from '@/core'; | ||
| export * from '@/features'; | ||
| export * from '@/scraper'; | ||
| export * from '@/spaces'; | ||
| export * from '@/timeline'; |
There was a problem hiding this comment.
💡 Codebase verification
Uncomment and keep the core module export
The @/core module contains essential utilities, API handling, authentication, and types that are heavily used throughout the codebase. The export should be uncommented to maintain a clean public API.
export { Scraper } from './scraper';
export * from '@/cli';
-// Not sure if this is needed
-// export * from '@/core';
+export * from '@/core';
export * from '@/features';
export * from '@/scraper';
export * from '@/spaces';
export * from '@/timeline';🔗 Analysis chain
Consolidate exports and remove unnecessary comments
The consolidation of exports in src/index.ts enhances the module's accessibility. However, the commented-out export of @/core indicates uncertainty.
- Confirm if re-exporting from
@/coreis necessary. If not, consider removing the comment to clean up the code.
Apply this diff to remove the unnecessary comment:
export { Scraper } from './scraper';
export * from '@/cli';
-// Not sure if this is needed
-// export * from '@/core';
export * from '@/features';
export * from '@/scraper';
export * from '@/spaces';
export * from '@/timeline';Ensure that all necessary exports are included and that the application functions as expected without the core exports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check if core module exists and show its contents
echo "=== Checking core module ==="
fd -t f "index.ts" -p "core/"
echo -e "\n=== Looking for @/core imports ==="
rg "@/core" --type ts
echo -e "\n=== Looking for package imports ==="
# This will help us understand if anyone is importing from the root package
rg "from '[^@].*'" --type ts
Length of output: 16242
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/index.ts (1)
1-1: Consider standardizing to absolute import path.For consistency with other imports and the PR's objective of using absolute paths, consider updating to
@/scraper. However, since this is the main entry point, the current relative import is also acceptable.-export { Scraper } from './scraper'; +export { Scraper } from '@/scraper';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.ts(1 hunks)
🔇 Additional comments (2)
src/index.ts (2)
1-9: Well-organized module structure!The export structure successfully implements a clean, modular API that aligns with the PR's objectives. The feature-based organization and consistent use of absolute imports (with one minor exception) will improve code maintainability.
3-8: Verify potential circular dependencies and build impact.While the module organization looks good, there are two concerns to address:
- Potential circular dependency between the individual
Scraperexport and@/scrapermodule export- Impact of these exports on the build process
Let's verify the import patterns and potential issues:
✅ Verification successful
Export structure is clean and well-organized
The verification shows:
- No circular dependencies between Scraper exports
- Consistent import patterns across the codebase
- Proper build configuration supporting the module structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential circular dependencies and import patterns echo "=== Checking for circular dependencies ===" # Look for imports of Scraper in scraper module rg "import.*Scraper.*from.*['\"]@/scraper['\"]" --type ts echo -e "\n=== Analyzing import patterns ===" # Check if any files are importing both individual Scraper and from @/scraper rg "import.*Scraper.*from" --type ts echo -e "\n=== Verifying build configuration ===" # Check if rollup config handles these exports correctly fd -g "rollup.config.*" -x cat {}Length of output: 3105
|
okay impossible to review! please send progressively in smaller PRs |
Refactor: Reorganize Project Structure with Modular Design
Reorganizes codebase with a modular, feature-based architecture:
Changes:
Example changes:
Testing Strategy:
Summary by CodeRabbit
Release Notes
Project Restructuring
@/alias.index.tsfiles.Code Organization
Build Configuration
Maintenance