improve code #490

Closed
opened 2026-02-16 17:27:03 -05:00 by yindo · 1 comment
Owner

Originally created by @Bill234 on GitHub (Jul 4, 2025).

You’ve made a strong case, and based on both your comments and the code in packages/opencode/src/session/index.ts, it’s reasonable to call this code “truly horrific” from a software engineering perspective. Here’s a concise summary of why many engineers would agree with you:


Why This Code Is Problematic

1. Monolithic Design

  • The file is extremely large and tries to do everything related to sessions, making it hard to reason about or maintain.

2. Poor Separation of Concerns

  • Session state, persistence, business logic, tool execution, and event handling are all mixed together.
  • UI and domain logic are tightly coupled (e.g., toUIMessage, toParts).

3. Hard to Test and Extend

  • Functions have a lot of side effects and dependencies.
  • Adding a new feature or fixing a bug risks breaking unrelated parts of the code.

4. State Management Issues

  • Global mutable state (App.state) is manipulated from many places, which can easily lead to bugs and race conditions.

5. Huge Functions

  • Functions like chat() do far too much, violating the single responsibility principle.

6. Error Handling

  • Inconsistent and often buried in the middle of large, complex code paths.

“Truly Horrific” Is Justified

Most experienced engineers would call this code a “code smell” or even a “code horror” due to:

  • Lack of modularity.
  • High complexity.
  • Poor maintainability.
  • Difficult testability.
  • Blurring of architectural boundaries.

Summary:
Your assessment is fair. This is a classic example of what to avoid in production code. If you want a specific refactoring suggestion or want to see an example of how a part of this could be improved, just ask!

Originally created by @Bill234 on GitHub (Jul 4, 2025). You’ve made a strong case, and based on both your comments and the code in `packages/opencode/src/session/index.ts`, it’s reasonable to call this code “truly horrific” from a software engineering perspective. Here’s a concise summary of why many engineers would agree with you: --- ### Why This Code Is Problematic **1. Monolithic Design** - The file is extremely large and tries to do everything related to sessions, making it hard to reason about or maintain. **2. Poor Separation of Concerns** - Session state, persistence, business logic, tool execution, and event handling are all mixed together. - UI and domain logic are tightly coupled (e.g., `toUIMessage`, `toParts`). **3. Hard to Test and Extend** - Functions have a lot of side effects and dependencies. - Adding a new feature or fixing a bug risks breaking unrelated parts of the code. **4. State Management Issues** - Global mutable state (`App.state`) is manipulated from many places, which can easily lead to bugs and race conditions. **5. Huge Functions** - Functions like `chat()` do far too much, violating the single responsibility principle. **6. Error Handling** - Inconsistent and often buried in the middle of large, complex code paths. --- ### “Truly Horrific” Is Justified Most experienced engineers would call this code a “code smell” or even a “code horror” due to: - Lack of modularity. - High complexity. - Poor maintainability. - Difficult testability. - Blurring of architectural boundaries. --- **Summary:** Your assessment is fair. This is a classic example of what to avoid in production code. If you want a specific refactoring suggestion or want to see an example of how a part of this could be improved, just ask!
yindo closed this issue 2026-02-16 17:27:03 -05:00
Author
Owner

@thdxr commented on GitHub (Jul 4, 2025):

the code in that file is bad yes but it's intentional - it's the most complex part of the app where we're still learning how best to do things. once things settle down we'll have a better perspective on how to refactor

once you get more experience things like that will be more clear to you

@thdxr commented on GitHub (Jul 4, 2025): the code in that file is bad yes but it's intentional - it's the most complex part of the app where we're still learning how best to do things. once things settle down we'll have a better perspective on how to refactor once you get more experience things like that will be more clear to you
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: anomalyco/opencode#490