Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .claude/agent-memory/senior-enterprise-java/MEMORY.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
- [User Role and Expertise](user_role.md) — senior Java developer on open-daimon, expects clean architecture and proper module boundaries
- [RAG storage refactor](project_rag_storage_refactor.md) — RAG documentIds moved from thread.memoryBullets to message.metadata; handler update still pending
- [Migration files live in opendaimon-common](feedback_migration_location.md) — core migrations in opendaimon-common, not opendaimon-app; next free version from glob there
- [Install updated modules before testing dependents](feedback_test_classpath.md) — mvnw install -pl <module> -DskipTests needed when shared module changed
- [Agent guard: use @ConditionalOnBean(AgentExecutor.class)](feedback_conditional_on_bean_for_agent_guard.md) — cleaner than stacking @ConditionalOnProperty for agent.enabled in handler beans
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
name: Use @ConditionalOnBean(AgentExecutor.class) as agent-enabled guard in handlers
description: For command handlers that require agent module, use @ConditionalOnBean(AgentExecutor.class) instead of @ConditionalOnProperty for agent.enabled
type: feedback
---

`AgentExecutor` bean is only created when `open-daimon.agent.enabled=true`. To guard a handler bean on agent being enabled, use `@ConditionalOnBean(AgentExecutor.class)` — cleaner and semantically correct compared to a second `@ConditionalOnProperty` which can have stacking/ordering issues.

**Why:** Spring Boot `@ConditionalOnProperty` is repeatable but when stacking two on the same method for unrelated prefixes, the behavior can be surprising. `@ConditionalOnBean` expresses the real dependency and is unambiguous.

**How to apply:** Command handlers only valid when agent module is active should declare `@ConditionalOnBean(AgentExecutor.class)` alongside their `@ConditionalOnProperty` for the command toggle.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
name: Migration files live in opendaimon-common, not opendaimon-app
description: DB migrations for the core "user" table are in opendaimon-common/src/main/resources/db/migration/core/, not opendaimon-app
type: feedback
---

Core DB migrations (user table, agent tables, etc.) live in `opendaimon-common/src/main/resources/db/migration/core/`, not in `opendaimon-app`. The plan said `opendaimon-app` but inspection confirmed the correct location.

**Why:** Flyway is configured per module; common migrations travel with `opendaimon-common`.

**How to apply:** When adding a migration for a base entity, always glob `opendaimon-common/**/migration/core/V*.sql` to find the next free version number.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
name: Run mvnw install on updated modules before testing dependent modules
description: When a module dependency is modified, install it first; otherwise test compilation of dependent modules will use the stale JAR from the local Maven repo
type: feedback
---

`./mvnw test -pl opendaimon-telegram` uses the installed JAR of `opendaimon-common` from `~/.m2`. If `opendaimon-common` was just modified, run `./mvnw install -pl opendaimon-common -DskipTests` first, otherwise test compilation in `opendaimon-telegram` will see stale symbols.

**Why:** Maven test classpath resolution for single-module builds uses installed artifacts, not reactor targets.

**How to apply:** After editing any shared module (`opendaimon-common`, `opendaimon-bulkhead`, etc.), always `mvnw install -pl <module> -DskipTests` before running tests in a dependent module without `-am`.
26 changes: 12 additions & 14 deletions .claude/agents/senior-enterprise-java.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: senior-enterprise-java
description: "Senior Java engineer for multi-module Spring Boot work — analyzing services, writing tests, debugging module behavior in opendaimon-* modules. Use proactively for non-trivial Java coding tasks (>1 file, requires a test, or touches JPA/Spring config). Do NOT use for pure docs/config edits or questions answerable without touching code."
description: "Senior Java engineer for multi-module Spring Boot changes that span >=3 Java files, introduce a new service/entity/migration, or require new unit+integration test coverage in opendaimon-* modules. Orchestrator may handle simpler edits directly. Do NOT invoke for: single-file edits with <50 changed lines; bug fixes with user-supplied logs where the root-cause skill fits; docs-only or config-only changes; continuation of work the orchestrator has already started."
model: opus
color: blue
---
Expand All @@ -13,19 +13,17 @@ You are a senior Java engineer on `open-daimon` — a multi-module Java 21 / Spr
2. If Serena reports `Active Project: None`, call `activate_project("open-daimon")` before any symbolic lookup.
3. Open the target module's `*_MODULE.md` (e.g. `opendaimon-spring-ai/SPRING_AI_MODULE.md`) and the matching `docs/usecases/*.md` if the change touches a documented use case.

## Non-negotiable project conventions

- **Beans:** explicit `@Bean` methods in `config/` classes — never `@Service` / `@Component` / `@Repository` auto-scan. Use `ObjectProvider` for optional beans, `@Lazy` to break cycles at creation.
- **Config:** `@ConfigurationProperties` + `@Validated`; all values required in `application.yml` (no defaults in code). Wrapper types (`Integer`, `Boolean`, `Double`). Namespace `open-daimon.*`; toggles `*.enabled`.
- **Feature toggles:** `FeatureToggle.Module` / `.Feature` / `.TelegramCommand` constants — never raw strings in `@ConditionalOnProperty`.
- **AI calls:** always via `PriorityRequestExecutor` (never call AI services directly). Priorities: ADMIN / VIP / REGULAR.
- **Metrics:** via `OpenDaimonMeterRegistry`, format `<module>.<action>.<metric>`.
- **Entities:** base (`User`, `Message`) live in `opendaimon-common`. JPA inheritance — JOINED for `User` (discriminator `user_type`), SINGLE_TABLE for `Message` (discriminator `message_type`, metadata JSONB). `@PrePersist` / `@PreUpdate` for timestamps.
- **Packages:** `io.github.ngirchev.opendaimon.<module>.<layer>`.
- **Services:** `Foo` interface + `FooImpl`, `@RequiredArgsConstructor`, `@Slf4j`. Lombok and Vavr are preferred.
- **Language:** code, comments, javadoc, log and exception messages — English only. User-facing strings may be i18n.
- **Migrations:** `opendaimon-app/src/main/resources/db/migration/<module>/V<n>__<desc>.sql`, `IF NOT EXISTS`, `TIMESTAMP WITH TIME ZONE`, index FKs.
- **pom.xml:** dependency order = project modules → Spring → DB → utilities → test. All versions in `<properties>`. Never add a dependency without approval.
## Style & conventions — loaded by path, do not re-duplicate here

Full rules live in these files, already in context by the time you run:

- `AGENTS.md § Project Style Guide` — beans, services, entities, migrations, metrics, pom order.
- `.claude/rules/java/coding-style.md` — auto-loads for any `*.java` file.
- `.claude/rules/java/testing.md` + `.../testcontainers.md` — test expectations.
- `.claude/rules/java/security.md` — when touching auth/input/external IO.
- The module's `*_MODULE.md` (e.g. `opendaimon-telegram/TELEGRAM_MODULE.md`) — module-specific behavior.

Your step 1 stays: read these before writing code. Do not paraphrase them into your output — just follow them.

## Discovery tools — prefer over ad-hoc search

Expand Down
1 change: 1 addition & 0 deletions .claude/rules/java/fixture-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ paths:
- `auto-mode-model-selection.md` -> `AutoModeModelSelectionFixtureIT`
- `text-pdf-rag.md` -> `TextPdfRagFixtureIT`
- `image-pdf-vision-cache.md` -> `ImagePdfVisionCacheFixtureIT`
- `agent-image-attachment.md` -> `TelegramAgentImageFixtureIT`

Before modifying fixture tests, read the corresponding use case doc from `docs/usecases/`.

Expand Down
17 changes: 17 additions & 0 deletions .claude/rules/java/telegram-module.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,20 @@ paths:
# Telegram Module

Before modifying Telegram module behavior, read `opendaimon-telegram/TELEGRAM_MODULE.md`.

## Group Chat Conceptual Model

In this project a **group chat is treated as a single logical participant**, not as a set of individuals. All state that Telegram scopes per-chat — conversation history, current model, current language for the bot menu, command menu snapshot, assistant role, agent mode, thinking mode, recent models — belongs to a dedicated `TelegramGroup` entity (JOINED-inheritance subclass of `User`, discriminator `TELEGRAM_GROUP`) and every participant of the group shares it. There is no per-user-inside-group isolation.

Practical consequences — apply these every time you touch Telegram code:

1. **Scope key is always `chat_id`, never `user.telegramId`.** In private chats they happen to be equal (Telegram uses the user id as the chat id), in groups they diverge (group `chat_id` is negative, e.g. `-1001234567890`). Code that keys on `user.telegramId` works in private but silently breaks in groups.
2. **Per-chat Telegram state** (in-memory cache of which chats we already synced the command menu to, etc.) must be keyed on `chat_id` — typically the value returned by `update.getMessage().getChatId()` or `command.telegramId()` (whose field name is misleading — it stores the chat id, see `TelegramCommand.java`).
3. **Settings belong to the chat entity, not the invoker.** `/language`, `/model`, `/mode`, `/thinking`, `/role` all write to the resolved *settings owner* — a `TelegramGroup` row in group chats, the invoker's `TelegramUser` row in private chats. Resolution happens once per update in `TelegramBot.mapToTelegram*` via `ChatSettingsOwnerResolver.resolveForChat(chat, invoker)`; the result is stamped on `TelegramCommand.settingsOwner` and consumed by handlers through `ChatSettingsService`. Do NOT key settings writes on `cq.getFrom().getId()` or `user.telegramId` — that reintroduces per-invoker leakage (the original Bug #114 pattern).
4. **Adding a new chat-scoped setting?** Add the field to `User` (inherited by both `TelegramUser` and `TelegramGroup`) and route reads/writes through `ChatSettingsService` over a `User owner`. Never introduce a path that reads/writes the field only on `TelegramUser`.
5. **`BotCommandScopeChat`** with the group `chat_id` overrides Default scope for the whole group. `BotCommandScopeChatMember` (per-user-in-chat) is NOT used — it contradicts the shared-chat model. Menu version hash lives on whichever owner resolved for the chat; `TelegramBotMenuService.reconcileMenuIfStale(User owner, Long chatId)` dispatches hash read/write by subtype and persists via `ChatSettingsService`.
6. **Routing filter** (group/supergroup → process only `/cmd@bot`, reply-to-bot, or explicit self-mention) is separate from this model: it decides *whether* to process, not *whom* the state belongs to. See "Group/Supergroup Routing Policy" in `TELEGRAM_MODULE.md`.
7. **Exceptions to the "group = single participant" rule:** the FSM `TelegramUserSession.botStatus` (pending-input state, e.g. "awaiting custom role text") stays per-invoker so one member's `/role custom` flow does not eat another member's text. Whitelist / priority (admin/vip/regular) is also per-invoker — groups have no access level; their members do.
8. **Cross-module summarization lookup:** `SummarizationService` (in `opendaimon-common`) resolves the chat-scoped preferredModelId via `ChatOwnerLookup.findByChatId(thread.scopeId)` — a common-module SPI bound by `TelegramChatOwnerLookup` in the telegram module. This guarantees the group's picked model lands in `ChatAICommand.metadata` and prevents the HTTP 400 "model is required" regression.

If a change appears to require per-user-inside-group state for any other field (e.g. "each member gets their own history in the group"), stop — that is a different product decision and must be discussed with the user before implementation.
59 changes: 59 additions & 0 deletions .claude/rules/java/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,68 @@ paths:
2. Target 80%+ line coverage (JaCoCo)
3. Focus on service and domain logic — skip trivial getters/config classes

## Mandatory Test Coverage

Every bug fix and every new feature is incomplete without a test that pins the new behavior:

- **Bug fix** — add a regression test that fails on the original code and passes after the fix. Place it next to the existing tests of the modified service, name it `shouldDoXWhenY` describing the corrected behavior, and reference the originating review comment / issue in a brief comment so the intent survives future refactors.
- **New feature** — add unit tests for each new public method on the service layer. If the feature carries data into an LLM (vision, RAG, tool-calling, conversation memory), follow the layering rule below: unit + fixture IT minimum, plus a manual IT when an LLM round-trip is the only proof the wiring works.
- **No test, no merge.** A change that only edits production code without test coverage is not finished — even if it compiles and the manual smoke check passes. The test is the artifact that prevents the same bug from coming back six months later when the surrounding code has shifted.

## Project Conventions

- **JUnit 5** + **AssertJ** + **Mockito** + **Testcontainers**
- Test naming: `shouldDoSomethingWhenCondition`
- Mirror `src/main/java` package structure in `src/test/java`
- Fix implementation, not tests (unless tests are wrong)

## Maven multi-module gotcha

When you change a class in a shared module (e.g. `opendaimon-common`) and run
tests in a downstream module, **always pass `-am` (also-make)**:

```sh
./mvnw test -pl opendaimon-spring-ai -am -Dtest=MyTest
```

Without `-am`, Maven uses the previously-installed JAR / `target/classes` of
the upstream module and silently runs tests against the **stale** version of
the changed class. Symptom: compile errors like

```
constructor MyClass cannot be applied to given types;
required: 5 args; found: 6 args
```

even though the source file in the upstream module clearly has the 6-arg
constructor — Maven just hasn't recompiled it.

When in doubt, run `./mvnw clean compile` over the whole reactor first, then
the targeted `test -pl ... -am` run.

Also, when targeting a single test in a multi-module build, surefire fails on
sibling modules where that test name does not exist. Add
`-Dsurefire.failIfNoSpecifiedTests=false` to make surefire skip those modules
quietly instead of failing the build.

## Test layers — when to use what

The project keeps three layers of tests; pick the right one before you start
writing.

| Layer | Path | Models | When |
|---|---|---|---|
| **Unit** | `*/src/test/java/**` | mocks (`when(chatModel.stream(...))`) | Every public method on a service. Fast, deterministic, runs on every commit. |
| **Fixture IT** | `opendaimon-app/src/it/java/**/fixture/` (`@Tag("fixture")`) | mocks or deterministic stubs | One per use case in `docs/usecases/`. Wires real Spring components together but never calls a real LLM — keeps `-Pfixture` fast and reliable. |
| **Manual IT** | `opendaimon-app/src/it/java/**/manual/` (`@Tag("manual")` + `@EnabledIfSystemProperty(...)`) | **real Ollama** (local) and/or **real OpenRouter** | End-to-end behavior of the same use case against a real LLM. Both flavors are usually present in pairs (`*OllamaManualIT`, `*OpenRouterManualIT`). Not in CI. |

Rule of thumb: if a use case carries data through to an LLM (vision, RAG,
tool-calling, conversation memory), it needs a manual IT in addition to the
unit + fixture coverage. Mocks pass the test even when the production wiring
silently drops the data; only a real LLM proves the model actually received it.

When the use case targets a vision-capable code path, prefer **OpenRouter**
with an explicit vision model (`z-ai/glm-4.5v`, `google/gemini-2.5-flash-preview`)
over `openrouter/auto` — auto-routing picks unpredictable models and produces
flaky test results. The Ollama variant should use a small local vision model
(`gemma3:4b`) and gate on `manual.ollama.e2e=true`.
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@

## Subagent delegation

- For non-trivial Java changes in `opendaimon-*` modules (>1 file, requires a test, or touches JPA/Spring config), delegate to the `senior-enterprise-java` subagent.
- Do not spawn a subagent for a one-line fix you can make directly.
- Delegate to `senior-enterprise-java` only when the task meets one of: >=3 Java files changed, a new service/entity/migration, OR both unit and integration test coverage required. For smaller changes handle them in the main loop.
- Never delegate: single-file edits <50 lines, log-driven bug fixes (use `root-cause` skill), docs-only or config-only changes, continuation of in-progress work.
- Before delegating, state in one sentence why the threshold is met.

## Debugging

Expand Down
Loading