feat(go): complete Go rewrite with ent, fx, echo, bubbletea, gotd/td#563
feat(go): complete Go rewrite with ent, fx, echo, bubbletea, gotd/td#563luoling8192 wants to merge 12 commits intomainfrom
Conversation
Full Go rewrite of the Telegram message search codebase preserving the core takeout and embedding search features. Stack: - entgo/ent v0.14.4 for ORM (auto-migration + generated client) - go.uber.org/fx v1.24.0 for dependency injection - labstack/echo v4 for HTTP + bot webhook - gotd/td v0.115.0 for MTProto (confined to tgclient package) - go-telegram/bot v1.14.0 for Telegram Bot API - charmbracelet/bubbletea v1.3.4 for TUI - pgx/v5 + pgvector for raw SQL and vector search - go-openai for embedding API (OpenAI-compatible) - viper + zap for config and structured logging Architecture: - tgclient.API interface wraps all gotd/td types, preventing leakage - sync, tui, bot packages only depend on the interface - Vector columns managed via raw SQL outside ent schema - Webhook and long-polling modes supported for bot - fx.Module per package for clean wiring in cmd/server and cmd/tui Binaries: - cmd/server: Echo HTTP server + Telegram bot - cmd/tui: interactive terminal for auth, sync, and search https://claude.ai/code/session_01A57Vi2yjqv9vRbEdA5NrBN
Summary of ChangesHello @luoling8192, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a significant architectural shift, replacing the existing Telegram message search system with a robust and high-concurrency Go implementation. It introduces a modular design, separating concerns into distinct services for configuration, database interaction, embedding generation, search logic, Telegram client communication, and user interfaces. The new system is designed for scalability and maintainability, offering both a web-based bot interface and a terminal-based interactive tool. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive rewrite of the application in Go. The architecture is well-structured, leveraging modern libraries like fx for dependency injection, ent for ORM, and echo for the web server. The code is generally clean and demonstrates good practices. My review focuses on a few areas for improvement, including configuration, error handling, graceful shutdown, and some incomplete functionality that could lead to bugs. Overall, this is a very strong foundation for the project.
| func register(lc fx.Lifecycle, b *Bot, e *echo.Echo) { | ||
| lc.Append(fx.Hook{ | ||
| OnStart: func(ctx context.Context) error { | ||
| if b.cfg.WebhookURL != "" { | ||
| return b.startWebhook(ctx, e) | ||
| } | ||
| return b.startPolling(ctx) | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
The DeleteWebhook method is defined but never called. When the server shuts down, the webhook registration on Telegram's side will persist. This can lead to Telegram sending updates to a dead endpoint, resulting in failed delivery notifications and potential rate-limiting. You should add an OnStop hook to the register function's lifecycle to call b.DeleteWebhook(ctx) for a clean shutdown.
func register(lc fx.Lifecycle, b *Bot, e *echo.Echo) {
lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
if b.cfg.WebhookURL != "" {
return b.startWebhook(ctx, e)
}
return b.startPolling(ctx)
},
OnStop: func(ctx context.Context) error {
if b.cfg.WebhookURL != "" {
b.log.Info("deleting webhook")
return b.DeleteWebhook(ctx)
}
return nil
},
})
}| // GetHistory fetches up to limit messages for the given chat, offset by offsetID. | ||
| func (c *Client) GetHistory(ctx context.Context, chatID string, offsetID, limit int) ([]TGMessage, error) { | ||
| // NOTE: A full implementation resolves chatID to the correct InputPeer type | ||
| // and access hash from the DB entity cache. This stub returns an empty result | ||
| // until the entity cache is wired up. | ||
| _ = chatID | ||
|
|
||
| result, err := c.api.MessagesGetHistory(ctx, &tg.MessagesGetHistoryRequest{ | ||
| Peer: &tg.InputPeerEmpty{}, | ||
| OffsetID: offsetID, | ||
| Limit: limit, | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("tgclient: get history: %w", err) | ||
| } |
There was a problem hiding this comment.
The GetHistory function appears to be a stub that uses &tg.InputPeerEmpty{}. This will prevent it from fetching history for any specific chat, as a concrete InputPeer is required. The implementation needs to resolve the chatID to a proper tg.InputPeer with its corresponding access hash, likely by looking it up from the database where chat information is stored. Without this, the message sync feature will not work correctly.
| // Spawn a goroutine to keep draining the channel and sending messages. | ||
| go func() { | ||
| for p := range ch { | ||
| _ = p // NOTE: In a real implementation send via a tea.Program.Send(). |
There was a problem hiding this comment.
The goroutine created to drain the progress channel from the sync service is not correctly sending messages back to the Bubble Tea runtime. The line _ = p discards the progress updates. To fix this, you need a way to send tea.Msg to the program, for example by having the command return a function that receives the tea.Program instance to call its Send method. As it is, only the first progress message is displayed, and the rest are lost, making the progress view non-functional.
| ignorePackageGlobs: | ||
| - 'github.com/groupultra/telegram-search/*' |
There was a problem hiding this comment.
The wrapcheck linter is enabled, but its checks are then disabled for the entire project via ignorePackageGlobs. This configuration makes the linter ineffective. If the goal is to enforce error wrapping, this ignore rule should be removed. If you're planning to adopt it incrementally, consider using //nolint:wrapcheck comments on a case-by-case basis instead of a global ignore.
# ignorePackageGlobs:
# - "github.com/groupultra/telegram-search/*"| log.Error("TUI exited with error", zap.Error(err)) | ||
| fmt.Fprintln(os.Stderr, "error:", err) | ||
| } | ||
| os.Exit(0) |
There was a problem hiding this comment.
Calling os.Exit(0) here causes an immediate, forceful termination of the process. This bypasses the fx application's graceful shutdown procedure, meaning OnStop hooks won't run. For a cleaner shutdown that allows for resource cleanup, you should inject fx.Shutdowner into runTUI and call shutdowner.Shutdown() when the TUI exits.
| Text: text, | ||
| }); err != nil { | ||
| // Log but don't propagate; send failures are non-fatal. | ||
| _ = err |
There was a problem hiding this comment.
The comment on line 193 states "Log but don't propagate", but the error is currently being discarded. While not propagating the error is reasonable here, it should be logged to help with debugging issues where the bot might be failing to send messages.
| _ = err | |
| log.Warn("failed to send message", zap.Int64("chat_id", chatID), zap.Error(err)) |
| ChatID: chatID, | ||
| Text: text, | ||
| ParseMode: models.ParseModeMarkdown, | ||
| }); err != nil { |
There was a problem hiding this comment.
The comment on line 203 states "Log but don't propagate", but the error is currently being discarded. While not propagating the error is reasonable here, it should be logged to help with debugging issues where the bot might be failing to send messages.
log.Warn("failed to send markdown message", zap.Int64("chat_id", chatID), zap.Error(err))| if len(filter.ChatIDs) == 0 { | ||
| return "" | ||
| } | ||
| return "AND c.chat_id = ANY($5)" |
There was a problem hiding this comment.
The SQL fragment AND c.chat_id = ANY($5) uses a hardcoded parameter index $5. This is brittle and can easily break if you add or remove parameters in the calling queries (vectorSearch and keywordSearch). Consider using named arguments with pgx (pgx.NamedArgs) or at least dynamically determining the parameter index to make this more robust.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95f8e70e7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log: log.Named("tgclient"), | ||
| ready: make(chan struct{}), | ||
| } | ||
| _ = lc |
There was a problem hiding this comment.
The constructor drops fx.Lifecycle (_ = lc) and there is no caller for (*Client).Run in the current Go app wiring, but c.api is initialized only inside Run. That means MTProto methods used by sync (GetDialogs/GetHistory) can execute with a nil API client after auth state is marked ready, causing runtime failures instead of a working sync flow.
Useful? React with 👍 / 👎.
| // NOTE: In a full implementation, resolve the accountID from the | ||
| // Telegram user ID via the DB (accounts table lookup). | ||
| // For now we use the user ID as a placeholder. | ||
| accountID := fmt.Sprintf("%d", userID) |
There was a problem hiding this comment.
The handler uses Telegram numeric userID as accountID, but the search SQL casts AccountID to UUID (owner_account_id = $1::uuid in internal/search/service.go). For normal bot users this causes invalid input syntax for type uuid, so /search requests fail consistently until this is mapped to a real account UUID.
Useful? React with 👍 / 👎.
| err := s.pool.QueryRow(ctx, ` | ||
| INSERT INTO chats (platform, chat_id, chat_name, owner_account_id) | ||
| VALUES ('telegram', $1, $2, | ||
| (SELECT id FROM accounts WHERE phone = $3 LIMIT 1)) |
There was a problem hiding this comment.
This query resolves owner_account_id via accounts.phone even though the function argument is named accountID and sync/search flows treat account identity as an ID. If callers pass a UUID, the subquery returns NULL and inserts fail on owner_account_id, so sync cannot persist chats/messages for UUID-based account references.
Useful? React with 👍 / 👎.
| go func() { | ||
| for p := range ch { | ||
| _ = p // NOTE: In a real implementation send via a tea.Program.Send(). | ||
| } |
There was a problem hiding this comment.
After returning the first progress event, the code drains the remaining channel messages and discards them (_ = p) instead of sending them back into Bubble Tea. As a result, the UI misses subsequent progress and completion/error signals, and the sync tab can remain stuck in a running state even when the background job has already finished.
Useful? React with 👍 / 👎.
Migrate all application-level logging from go.uber.org/zap to the
standard library log/slog (Go 1.21+), reducing one external dependency.
Changes:
- cmd/server: newLogger() returns *slog.Logger with JSON handler
- cmd/tui: newDevLogger() returns *slog.Logger with text handler at WARN
- fx wiring: fxevent.ZapLogger → fxevent.SlogLogger
- All internal packages: *zap.Logger → *slog.Logger, log.Named("x") → log.With("component", "x")
- tgclient: keep a minimal internal zap.Logger only for gotd/td's
telegram.Options.Logger (which requires *zap.Logger); documented with
a NOTICE comment. All other tgclient logging uses slog.
go.mod: remove go.uber.org/zap from the direct-deps block; it remains
as an indirect dep pulled in by go.uber.org/fx/fxevent (ZapLogger) and
gotd/td internals.
https://claude.ai/code/session_01A57Vi2yjqv9vRbEdA5NrBN
Co-authored-by: Neko <neko@ayaka.moe>
2 participants
Full Go rewrite of the Telegram message search codebase preserving
the core takeout and embedding search features.
Stack:
Architecture:
Binaries:
https://claude.ai/code/session_01A57Vi2yjqv9vRbEdA5NrBN