Skip to content

unify config#24

Open
tatchi wants to merge 1 commit intoygwyg:mainfrom
tatchi:unify-config
Open

unify config#24
tatchi wants to merge 1 commit intoygwyg:mainfrom
tatchi:unify-config

Conversation

@tatchi
Copy link
Copy Markdown
Contributor

@tatchi tatchi commented Feb 5, 2026

No description provided.

Comment thread dashboard/src/types.ts
// Custom ticker blacklist (insider trading restrictions, etc.)
ticker_blacklist?: string[]
}
import type { AgentConfig } from "../../src/schemas/agent-config"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it ok to import outside of dashboard folder ?

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR consolidates three duplicate AgentConfig interface definitions into a single source of truth using Zod schema validation in src/schemas/agent-config.ts.

Key changes:

  • Removed duplicate ~60-line AgentConfig interface from mahoraga-harness.ts
  • Removed duplicate ~50-line Config interface from dashboard/src/types.ts
  • Added three new required fields: llm_min_hold_minutes, allowed_exchanges, and starting_equity
  • Added comprehensive inline documentation to all schema fields
  • Updated tests to include new required fields

Benefits:

  • Eliminates code duplication and maintenance burden
  • Provides runtime validation via Zod schema
  • Single source of truth for configuration across backend and frontend
  • Type safety maintained via z.infer<typeof AgentConfigSchema>

Backward compatibility:
The change from optional to required fields is safe because the backend always initializes with DEFAULT_CONFIG (which includes all fields) and updates are partial merges via { ...this.state.config, ...body }.

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The refactoring eliminates code duplication by consolidating three identical config definitions into one centralized schema. All tests were updated to include new required fields. Backward compatibility is maintained because the backend always starts from DEFAULT_CONFIG with all fields present, and config updates use partial merging.
  • No files require special attention

Important Files Changed

Filename Overview
src/schemas/agent-config.ts Added comprehensive inline documentation and three new required fields (llm_min_hold_minutes, allowed_exchanges, starting_equity) to centralize configuration schema with runtime validation
dashboard/src/types.ts Replaced 50+ line duplicate Config interface with import from centralized AgentConfig schema, eliminating maintenance burden
src/durable-objects/mahoraga-harness.ts Removed duplicate 60-line AgentConfig interface definition, now imports from centralized schema
src/schemas/agent-config.test.ts Updated test fixture to include three newly required fields (llm_min_hold_minutes, allowed_exchanges, starting_equity) and added explicit return type annotation

Sequence Diagram

sequenceDiagram
    participant Dashboard as Dashboard<br/>(dashboard/src/types.ts)
    participant DO as MahoragaHarness<br/>(durable-objects/mahoraga-harness.ts)
    participant Schema as AgentConfigSchema<br/>(schemas/agent-config.ts)
    
    Note over Dashboard,Schema: Before: 3 duplicate Config definitions
    Note over Dashboard,Schema: After: 1 centralized source of truth
    
    Dashboard->>DO: POST /config (partial update)
    DO->>DO: Merge with DEFAULT_CONFIG
    Note over DO: All required fields present<br/>(llm_min_hold_minutes,<br/>allowed_exchanges,<br/>starting_equity)
    DO->>Schema: Validate with AgentConfigSchema
    Schema-->>DO: ✓ Valid AgentConfig
    DO->>DO: Persist to Durable Object storage
    DO-->>Dashboard: Return complete config
    
    Note over Dashboard,Schema: Type safety maintained across<br/>backend and frontend
Loading

@stritt
Copy link
Copy Markdown
Contributor

stritt commented Feb 10, 2026

This PR now has merge conflicts after merging #26, #27, and #23. Could you rebase against main? The config schema changes in those PRs likely overlap with this unification work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants