refactor(semantic): finalize prompt engine semantic layer architecture#180
Open
refactor(semantic): finalize prompt engine semantic layer architecture#180
Conversation
What changed: - Restore clear domain vs application boundaries and align responsibilities across layers. - Replace null-to-default-new patterns with explicit construction and stricter initialization contracts. - Split ObjectiveMappingRegistry responsibilities and consolidate ActionGroup handling through a single PromptSpecFactory path. - Apply fail-fast semantics for category seeding and ActionGroup resolution; remove silent drops and empty-map masking. - Formalize strict (require) vs permissive (find) APIs across semantic resolution. - Remove static IntentDefinitionDataSource and IntentDictionary entry points in favor of injectable providers. Why: - The semantic layer mixed persistence concerns with application orchestration, hid misconfiguration behind defaults, and allowed inconsistent ActionGroup wiring. Static singletons and silent fallbacks made behavior environment-dependent and hard to test or evolve safely. Impact: - Failures surface at startup or at the boundary where data is wrong, improving operational predictability. - Dependency injection makes components testable in isolation and keeps a single source of truth for intent and action semantics. - Tighter contracts reduce duplicate code paths and make future schema or registry changes localized. Constraints: - No intentional change to external product-facing prompt text or user-visible API contracts beyond stricter validation and earlier failure modes.
개요이 변경 사항은 개요IntentDictionary를 정적 레지스트리에서 인스턴스 기반 설계로 변환하고, 여러 클래스들을 의존성 주입을 통해 필요한 서비스를 받도록 리팩토링합니다. 또한 CanonicalActionRegistry, CategorySemanticProfile 및 관련 공급자들에 엄격한 유효성 검사를 추가합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Config as Spring Config
participant Assembly as IntentDefinitionProviderAssembly
participant DataSource as IntentDefinitionDataSource
participant Dictionary as IntentDictionary
participant Resolver as IntentDefaultsResolver
participant Client as Client Code
Config->>Assembly: productionIntentDictionary()
Assembly->>Assembly: productionDefinitionsProviders()
Assembly->>Assembly: productionResolutionDefaultsProviders()
Assembly->>DataSource: new(providers, defaults providers)
Assembly->>Dictionary: new(dataSource)
Dictionary->>DataSource: collectDefinitionsEntries()
Dictionary->>DataSource: collectResolutionDefaultEntries()
Dictionary->>Dictionary: validateIntentCoverageCompleteness()
Dictionary-->>Assembly: Dictionary instance
Assembly-->>Config: Dictionary instance
Config->>Resolver: new(dictionary)
Config-->>Client: Bean with injected dependencies
Client->>Resolver: resolve(intent)
Resolver->>Dictionary: getResolutionDefaults(intent)
Dictionary-->>Resolver: IntentResolutionDefaults
Resolver-->>Client: IntentDefaults
예상 코드 검토 노력🎯 4 (복잡) | ⏱️ ~60분 관련 가능 PR
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/ConfirmedSemanticAxesFindActionGroupTest.java (1)
23-68:actionType이 존재하지만 해석 불가한 permissive 경로도 고정해 주세요.지금은
actionType이 비어 있는 경우와 정상 매핑되는 경우만 검증합니다.findActionGroup()의 핵심 계약은 값이 있어도 canonical mapping이 없으면 throw하지 않고Optional.empty()를 돌려주는 것이므로, 레지스트리에 없는 테스트 더블 하나를 넣어 그 경로도 같이 잠가 두는 편이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/ConfirmedSemanticAxesFindActionGroupTest.java` around lines 23 - 68, Add a third test to cover the permissive path where actionType is present but not mapped in the canonical registry: construct a ConfirmedSemanticAxes with actionType set to a test-only enum/value (or a fake WritingActionType test double) that is NOT registered in the DefaultCanonicalActionRegistry used in the test, then call axes.findActionGroup(canonical) and assert it returns Optional.empty() (and does not throw); reference ConfirmedSemanticAxes, actionType, findActionGroup, and DefaultCanonicalActionRegistry to locate where to add this case.sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/RoutingRuleEngineTest.java (1)
24-25: 라우팅 테스트가 production intent catalog에 불필요하게 결합됐습니다.이 스위트는 rule selection/우선순위만 검증하는데,
baseDefaults()가productionIntentDictionary()를 읽기 시작하면서 semantic 기본값 변경만으로도 테스트가 깨질 수 있습니다. 여기서는IntentDefaults를 테스트 안에서 고정 값으로 직접 만들어 두는 편이 실패 원인을 라우팅 로직으로 한정하기 좋습니다.Also applies to: 53-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/RoutingRuleEngineTest.java` around lines 24 - 25, The test is coupled to the production intent catalog via IntentDefinitionProviderAssembly.productionIntentDictionary(); replace that with a test-controlled dictionary/defaults by constructing a deterministic IntentDefaults instance (e.g., new/factory IntentDefaults with fixed values or a test IntentDefaults.of(...)) and use a test IntentDictionary built from that fixed defaults instead of calling productionIntentDictionary(); update the IntentDictionary field initialization and the other occurrences around the suite (the spots using baseDefaults()/productionIntentDictionary(), including the instances noted at 53-55) so the routing tests use the local fixed IntentDefaults and cannot break when production semantic defaults change.sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionaryTest.java (1)
42-50: 인스턴스 격리 테스트에서 불변성 검증 강화를 고려하세요.현재 테스트는 두 인스턴스가 동일 객체가 아님(
isNotSameAs)과 값 동등성(isEqualTo)만 검증합니다. 한 인스턴스 수정이 다른 인스턴스에 영향을 미치지 않는지 검증하는 mutation 테스트를 추가하면 "doNotShareMutableState" 의도를 더 강하게 표현할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionaryTest.java` around lines 42 - 50, Update the test distinctInstances_doNotShareMutableState in IntentDictionaryTest to actually mutate one IntentDictionary and verify the other remains unchanged: create two instances via IntentDefinitionProviderAssembly.productionIntentDictionary(), record the original value from a.getResolutionDefaults(ActionIntent.GENERATE).defaultObjective(), perform a non-destructive mutation on a (e.g., change its resolution defaults or defaultObjective via the public API used in tests), then assert that b.getResolutionDefaults(ActionIntent.GENERATE).defaultObjective() still equals the original recorded value and that a and b are not the same instance; this ensures IntentDictionary instances do not share mutable state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileRegistry.java`:
- Around line 86-88: The method toActionGroupMap currently returns an empty Map
when the actions map is null or empty, silently allowing an empty
actionsByIntent and bypassing the category-level fail-fast contract; change it
to validate inputs and fail-fast by throwing an IllegalArgumentException (or a
suitable runtime exception) when actions is null or actions.isEmpty(), including
the PromptCategory (category) in the exception message to aid debugging; also
double-check the prepare(...) call site to ensure it does not only iterate
actions.keySet() so compatibility overrides are still applied when actions map
is present, but the immediate fix is to make toActionGroupMap (in
DefaultCategorySemanticProfileRegistry) validate and throw rather than return
Map.of() for empty/null actions.
- Around line 127-140: The method requireActionGroupForCategorySeed should guard
against a null actionType before calling
canonicalActionRegistry.requireActionGroup; add a null-check at the start of
requireActionGroupForCategorySeed (for PromptCategory category, ActionIntent
intent, ActionTypeInterface actionType) and throw an IllegalArgumentException
that includes category.name(), intent.name(), and a clear indication that
actionType is null (or include actionType.key() when non-null) so callers get
consistent seed-diagnostic context instead of a raw NPE from
canonicalActionRegistry.requireActionGroup.
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionProviderAssembly.java`:
- Around line 17-39: Ensure all ActionIntent categories are covered by the
providers by adding a fail-fast validation in IntentDefinitionProviderAssembly:
when building productionDefinitionsProviders() and
productionResolutionDefaultsProviders() (or when constructing IntentDictionary),
enumerate ActionIntent.values(), derive their category, and verify that every
category has a corresponding IntentDefinitionEntriesProvider and
IntentResolutionDefaultsEntriesProvider; if any category is missing, throw an
IllegalStateException with a clear message listing missing categories so
misconfiguration is detected immediately (use the existing
productionDefinitionsProviders, productionResolutionDefaultsProviders and
IntentDictionary construction paths to locate where to run this validation).
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionary.java`:
- Around line 31-39: The current population loops in IntentDictionary silently
overwrite duplicate keys and accept null values; change the loops that iterate
dataSource.collectDefinitionsEntries() and
dataSource.collectResolutionDefaultEntries() so that before inserting into defs
or res you (1) verify e.definition() / e.defaults() is non-null and throw an
IllegalStateException if null, and (2) check if defs.containsKey(e.intent()) or
res.containsKey(e.intent()) and throw an IllegalStateException reporting the
duplicate intent and source entry; do this in the IntentDictionary constructor
(or the initialization method) so duplicates/nulls are rejected at creation time
rather than quietly overwriting and passing validateIntentCoverageCompleteness.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/CategorySemanticProfileSeedSourceAssemblerTest.java`:
- Around line 90-93: The test seed sets ActionIntent.REWRITE but reuses
original.fallbackCandidates, creating an inconsistent fixture; in
CategorySemanticProfileSeed construction inside
CategorySemanticProfileSeedSourceAssemblerTest update the fallbackCandidates to
match the new fallback intent (i.e., provide the rewrite-specific candidate
list) or stop overriding the fallback (keep original.fallbackCandidates) so the
seed remains consistent—ensure the change is made where new
CategorySemanticProfileSeed(...) is invoked and touches the fallbackCandidates
argument to align with ActionIntent.REWRITE.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/service/spec/PromptSpecFactoryTest.java`:
- Around line 103-105: The test currently doesn't verify action resolution
because objectiveResolver is stubbed to always return FACTUAL; update
PromptSpecFactoryTest so that after creating the spec via
factory.createFromConfirmedAxes(axes, "hello", null) you also assert
spec.getActionType() equals the expected action (e.g., axes.actionType() or the
concrete ActionType you set up in the test), ensuring createFromConfirmedAxes
actually uses the axes.actionType() rather than only relying on
objectiveResolver returning FACTUAL.
---
Nitpick comments:
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/RoutingRuleEngineTest.java`:
- Around line 24-25: The test is coupled to the production intent catalog via
IntentDefinitionProviderAssembly.productionIntentDictionary(); replace that with
a test-controlled dictionary/defaults by constructing a deterministic
IntentDefaults instance (e.g., new/factory IntentDefaults with fixed values or a
test IntentDefaults.of(...)) and use a test IntentDictionary built from that
fixed defaults instead of calling productionIntentDictionary(); update the
IntentDictionary field initialization and the other occurrences around the suite
(the spots using baseDefaults()/productionIntentDictionary(), including the
instances noted at 53-55) so the routing tests use the local fixed
IntentDefaults and cannot break when production semantic defaults change.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/ConfirmedSemanticAxesFindActionGroupTest.java`:
- Around line 23-68: Add a third test to cover the permissive path where
actionType is present but not mapped in the canonical registry: construct a
ConfirmedSemanticAxes with actionType set to a test-only enum/value (or a fake
WritingActionType test double) that is NOT registered in the
DefaultCanonicalActionRegistry used in the test, then call
axes.findActionGroup(canonical) and assert it returns Optional.empty() (and does
not throw); reference ConfirmedSemanticAxes, actionType, findActionGroup, and
DefaultCanonicalActionRegistry to locate where to add this case.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionaryTest.java`:
- Around line 42-50: Update the test distinctInstances_doNotShareMutableState in
IntentDictionaryTest to actually mutate one IntentDictionary and verify the
other remains unchanged: create two instances via
IntentDefinitionProviderAssembly.productionIntentDictionary(), record the
original value from
a.getResolutionDefaults(ActionIntent.GENERATE).defaultObjective(), perform a
non-destructive mutation on a (e.g., change its resolution defaults or
defaultObjective via the public API used in tests), then assert that
b.getResolutionDefaults(ActionIntent.GENERATE).defaultObjective() still equals
the original recorded value and that a and b are not the same instance; this
ensures IntentDictionary instances do not share mutable state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfd6446e-ae9c-48db-aa07-8669d5512a15
📒 Files selected for processing (28)
sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/IntentDefaultsResolver.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/application/semantic/IntentBasedAxisDefaultsResolver.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/common/enums/action/canonical/CanonicalActionRegistry.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/common/enums/action/canonical/DefaultCanonicalActionRegistry.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/CategorySemanticProfile.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/CategorySemanticProfileSeedSource.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/ConfirmedSemanticAxes.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionDataSource.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionEntriesProvider.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionProviderAssembly.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionary.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentResolutionDefaultsEntriesProvider.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileRegistry.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileSeedSource.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/service/spec/PromptSpecFactory.javasharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/infrastructure/config/PromptDomainConfig.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/GeneratePromptFromConfirmedAxesServiceTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/DomainFinalizerTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/IntentDefaultsResolverTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/application/engine/generation/legacy/RoutingRuleEngineTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/common/enums/action/canonical/CanonicalActionMappingTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/CategorySemanticProfileSeedSourceAssemblerTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/ConfirmedSemanticAxesFindActionGroupTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionDataSourceTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionaryTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileRegistryFailFastTest.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/policy/SemanticStructurePhase5Test.javasharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/service/spec/PromptSpecFactoryTest.java
Comment on lines
+86
to
88
| private Map<ActionIntent, List<ActionGroup>> toActionGroupMap( | ||
| PromptCategory category, Map<ActionIntent, List<ActionTypeInterface>> actions) { | ||
| if (actions == null || actions.isEmpty()) return Map.of(); |
There was a problem hiding this comment.
빈 actionsByIntent가 여전히 조용히 통과됩니다.
등록 대상 카테고리에서 actions == null || actions.isEmpty()면 빈 프로필이 그대로 만들어집니다. 게다가 prepare(...)도 actions.keySet()만 순회하므로 compatibility override로 복구되지 않아, 이번 PR이 의도한 카테고리 단위 fail-fast 계약이 여기서 우회됩니다.
🔧 제안 수정
private Map<ActionIntent, List<ActionGroup>> toActionGroupMap(
PromptCategory category, Map<ActionIntent, List<ActionTypeInterface>> actions) {
- if (actions == null || actions.isEmpty()) return Map.of();
+ if (actions == null || actions.isEmpty()) {
+ throw new IllegalArgumentException(
+ "Category semantic profile seed must define at least one intent/action mapping for category="
+ + category.name());
+ }
Map<ActionIntent, List<ActionGroup>> out = new HashMap<>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileRegistry.java`
around lines 86 - 88, The method toActionGroupMap currently returns an empty Map
when the actions map is null or empty, silently allowing an empty
actionsByIntent and bypassing the category-level fail-fast contract; change it
to validate inputs and fail-fast by throwing an IllegalArgumentException (or a
suitable runtime exception) when actions is null or actions.isEmpty(), including
the PromptCategory (category) in the exception message to aid debugging; also
double-check the prepare(...) call site to ensure it does not only iterate
actions.keySet() so compatibility overrides are still applied when actions map
is present, but the immediate fix is to make toActionGroupMap (in
DefaultCategorySemanticProfileRegistry) validate and throw rather than return
Map.of() for empty/null actions.
Comment on lines
+127
to
+140
| private ActionGroup requireActionGroupForCategorySeed( | ||
| PromptCategory category, ActionIntent intent, ActionTypeInterface actionType) { | ||
| try { | ||
| return canonicalActionRegistry.requireActionGroup(actionType); | ||
| } catch (IllegalArgumentException ex) { | ||
| throw new IllegalArgumentException( | ||
| "Category semantic profile seed assembly: cannot resolve ActionGroup for category=" | ||
| + category.name() | ||
| + ", intent=" | ||
| + intent.name() | ||
| + ", actionStableKey=" | ||
| + actionType.key(), | ||
| ex); | ||
| } |
There was a problem hiding this comment.
null action 항목은 현재 NPE로 새어 나갑니다.
여기서는 IllegalArgumentException만 감싸고 있어서, seed action list에 null이 들어오면 CanonicalActionRegistry.requireActionGroup(null)의 NullPointerException이 category/intent 정보 없이 그대로 올라옵니다. 시드 진단을 일관되게 유지하려면 이 지점에서 먼저 막는 편이 안전합니다.
🛡️ 제안 수정
private ActionGroup requireActionGroupForCategorySeed(
PromptCategory category, ActionIntent intent, ActionTypeInterface actionType) {
+ if (actionType == null) {
+ throw new IllegalArgumentException(
+ "Category semantic profile seed contains null action for category="
+ + category.name()
+ + ", intent="
+ + intent.name());
+ }
try {
return canonicalActionRegistry.requireActionGroup(actionType);
} catch (IllegalArgumentException ex) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/impl/DefaultCategorySemanticProfileRegistry.java`
around lines 127 - 140, The method requireActionGroupForCategorySeed should
guard against a null actionType before calling
canonicalActionRegistry.requireActionGroup; add a null-check at the start of
requireActionGroupForCategorySeed (for PromptCategory category, ActionIntent
intent, ActionTypeInterface actionType) and throw an IllegalArgumentException
that includes category.name(), intent.name(), and a clear indication that
actionType is null (or include actionType.key() when non-null) so callers get
consistent seed-diagnostic context instead of a raw NPE from
canonicalActionRegistry.requireActionGroup.
Comment on lines
+17
to
+39
| public static List<IntentDefinitionEntriesProvider> productionDefinitionsProviders() { | ||
| return List.of( | ||
| new IntentCreationIntentDefinitionsProvider(), | ||
| new IntentModificationIntentDefinitionsProvider(), | ||
| new IntentAnalysisIntentDefinitionsProvider(), | ||
| new IntentExplanationIntentDefinitionsProvider(), | ||
| new IntentPlanningIntentDefinitionsProvider(), | ||
| new IntentDecisionIntentDefinitionsProvider(), | ||
| new IntentResearchIntentDefinitionsProvider(), | ||
| new IntentExtractionIntentDefinitionsProvider()); | ||
| } | ||
|
|
||
| public static List<IntentResolutionDefaultsEntriesProvider> productionResolutionDefaultsProviders() { | ||
| return List.of( | ||
| new IntentCreationIntentResolutionDefaultsProvider(), | ||
| new IntentModificationIntentResolutionDefaultsProvider(), | ||
| new IntentAnalysisIntentResolutionDefaultsProvider(), | ||
| new IntentExplanationIntentResolutionDefaultsProvider(), | ||
| new IntentPlanningIntentResolutionDefaultsProvider(), | ||
| new IntentDecisionIntentResolutionDefaultsProvider(), | ||
| new IntentResearchIntentResolutionDefaultsProvider(), | ||
| new IntentExtractionIntentResolutionDefaultsProvider()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: ActionIntent enum 값 개수와 provider 개수 비교
echo "=== ActionIntent enum values ==="
rg -n "^\s+[A-Z_]+[,;]?\s*$" --type=java -A 0 | grep -E "ActionIntent\.java" | head -20
echo ""
echo "=== Counting providers in assembly ==="
rg -c "new Intent.*Provider\(\)" sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionProviderAssembly.javaRepository: rabitis99/prompts-backend
Length of output: 2627
🏁 Script executed:
#!/bin/bash
# Get the complete ActionIntent enum file
cat sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/common/enums/semantic/ActionIntent.javaRepository: rabitis99/prompts-backend
Length of output: 1893
🏁 Script executed:
#!/bin/bash
# Count the exact number of enum values in ActionIntent
rg "^\s+[A-Z_]+," sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/common/enums/semantic/ActionIntent.java | wc -lRepository: rabitis99/prompts-backend
Length of output: 71
ActionIntent enum의 모든 카테고리가 provider로 커버되는지 확인하세요.
ActionIntent enum은 28개의 값을 8개 카테고리로 구성하고 있으며, 8개의 definition providers와 8개의 resolution defaults providers가 각 카테고리를 담당합니다. 현재는 카테고리별 매핑이므로 커버되어 있으나, enum 값이 새로운 카테고리에 추가되거나 카테고리 구조가 변경될 경우 provider 누락 가능성이 있습니다. IntentDictionary 생성 시 fail-fast로 감지되지만, 코드 유지보수 시 주의가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDefinitionProviderAssembly.java`
around lines 17 - 39, Ensure all ActionIntent categories are covered by the
providers by adding a fail-fast validation in IntentDefinitionProviderAssembly:
when building productionDefinitionsProviders() and
productionResolutionDefaultsProviders() (or when constructing IntentDictionary),
enumerate ActionIntent.values(), derive their category, and verify that every
category has a corresponding IntentDefinitionEntriesProvider and
IntentResolutionDefaultsEntriesProvider; if any category is missing, throw an
IllegalStateException with a clear message listing missing categories so
misconfiguration is detected immediately (use the existing
productionDefinitionsProviders, productionResolutionDefaultsProviders and
IntentDictionary construction paths to locate where to run this validation).
Comment on lines
+31
to
+39
| Map<ActionIntent, IntentDefinition> defs = new HashMap<>(); | ||
| for (IntentDefinitionDataSource.IntentDefinitionEntry e : dataSource.collectDefinitionsEntries()) { | ||
| defs.put(e.intent(), e.definition()); | ||
| } | ||
|
|
||
| Map<ActionIntent, IntentResolutionDefaults> res = new HashMap<>(); | ||
| for (IntentDefinitionDataSource.IntentResolutionDefaultEntry e : dataSource.collectResolutionDefaultEntries()) { | ||
| res.put(e.intent(), e.defaults()); | ||
| } |
There was a problem hiding this comment.
중복/null 엔트리가 여전히 조용히 덮어써집니다.
두 루프가 put(...)만 사용해서 같은 ActionIntent가 두 번 수집되면 마지막 값으로 덮어써지고, null definition/default도 key coverage만 맞으면 validateIntentCoverageCompleteness(...)를 통과합니다. injectable provider 구조로 바뀐 지금은 이런 경우도 생성 시점에 막아야 초기화 결과가 결정적입니다.
🔒 제안 수정
Map<ActionIntent, IntentDefinition> defs = new HashMap<>();
for (IntentDefinitionDataSource.IntentDefinitionEntry e : dataSource.collectDefinitionsEntries()) {
- defs.put(e.intent(), e.definition());
+ Objects.requireNonNull(e, "definition entry");
+ ActionIntent intent = Objects.requireNonNull(e.intent(), "definition intent");
+ IntentDefinition definition = Objects.requireNonNull(e.definition(), "definition");
+ if (defs.putIfAbsent(intent, definition) != null) {
+ throw new IllegalStateException("Duplicate IntentDefinition for: " + intent);
+ }
}
Map<ActionIntent, IntentResolutionDefaults> res = new HashMap<>();
for (IntentDefinitionDataSource.IntentResolutionDefaultEntry e : dataSource.collectResolutionDefaultEntries()) {
- res.put(e.intent(), e.defaults());
+ Objects.requireNonNull(e, "resolution default entry");
+ ActionIntent intent = Objects.requireNonNull(e.intent(), "resolution default intent");
+ IntentResolutionDefaults defaults = Objects.requireNonNull(e.defaults(), "resolution defaults");
+ if (res.putIfAbsent(intent, defaults) != null) {
+ throw new IllegalStateException("Duplicate IntentResolutionDefaults for: " + intent);
+ }
}📝 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.
Suggested change
| Map<ActionIntent, IntentDefinition> defs = new HashMap<>(); | |
| for (IntentDefinitionDataSource.IntentDefinitionEntry e : dataSource.collectDefinitionsEntries()) { | |
| defs.put(e.intent(), e.definition()); | |
| } | |
| Map<ActionIntent, IntentResolutionDefaults> res = new HashMap<>(); | |
| for (IntentDefinitionDataSource.IntentResolutionDefaultEntry e : dataSource.collectResolutionDefaultEntries()) { | |
| res.put(e.intent(), e.defaults()); | |
| } | |
| Map<ActionIntent, IntentDefinition> defs = new HashMap<>(); | |
| for (IntentDefinitionDataSource.IntentDefinitionEntry e : dataSource.collectDefinitionsEntries()) { | |
| Objects.requireNonNull(e, "definition entry"); | |
| ActionIntent intent = Objects.requireNonNull(e.intent(), "definition intent"); | |
| IntentDefinition definition = Objects.requireNonNull(e.definition(), "definition"); | |
| if (defs.putIfAbsent(intent, definition) != null) { | |
| throw new IllegalStateException("Duplicate IntentDefinition for: " + intent); | |
| } | |
| } | |
| Map<ActionIntent, IntentResolutionDefaults> res = new HashMap<>(); | |
| for (IntentDefinitionDataSource.IntentResolutionDefaultEntry e : dataSource.collectResolutionDefaultEntries()) { | |
| Objects.requireNonNull(e, "resolution default entry"); | |
| ActionIntent intent = Objects.requireNonNull(e.intent(), "resolution default intent"); | |
| IntentResolutionDefaults defaults = Objects.requireNonNull(e.defaults(), "resolution defaults"); | |
| if (res.putIfAbsent(intent, defaults) != null) { | |
| throw new IllegalStateException("Duplicate IntentResolutionDefaults for: " + intent); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/main/java/org/example/sharedprompts/domain/prompt/domain/semantic/IntentDictionary.java`
around lines 31 - 39, The current population loops in IntentDictionary silently
overwrite duplicate keys and accept null values; change the loops that iterate
dataSource.collectDefinitionsEntries() and
dataSource.collectResolutionDefaultEntries() so that before inserting into defs
or res you (1) verify e.definition() / e.defaults() is non-null and throw an
IllegalStateException if null, and (2) check if defs.containsKey(e.intent()) or
res.containsKey(e.intent()) and throw an IllegalStateException reporting the
duplicate intent and source entry; do this in the IntentDictionary constructor
(or the initialization method) so duplicates/nulls are rejected at creation time
rather than quietly overwriting and passing validateIntentCoverageCompleteness.
Comment on lines
+90
to
+93
| return new CategorySemanticProfileSeed( | ||
| original.category(), original.taskDomain(), original.allowedIntents(), original.intentFitLevels(), | ||
| roles, original.actionsByIntent(), original.discouragedTonesByIntent(), original.discouragedStylesByIntent(), | ||
| ActionIntent.REWRITE, original.fallbackCandidates())); | ||
| ActionIntent.REWRITE, original.fallbackCandidates()); |
There was a problem hiding this comment.
테스트 시드가 fallback 기준과 후보 목록을 서로 다르게 만듭니다.
fallbackIntent는 REWRITE로 바꾸면서 fallbackCandidates는 원본을 재사용하고 있어서, 이 seed는 내부적으로 모순될 수 있습니다. assembler가 후보 목록도 함께 소비하면 잘못된 fixture로 테스트가 통과할 수 있으니, 후보 목록도 같이 갱신하거나 fallback 변경을 빼는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/semantic/CategorySemanticProfileSeedSourceAssemblerTest.java`
around lines 90 - 93, The test seed sets ActionIntent.REWRITE but reuses
original.fallbackCandidates, creating an inconsistent fixture; in
CategorySemanticProfileSeed construction inside
CategorySemanticProfileSeedSourceAssemblerTest update the fallbackCandidates to
match the new fallback intent (i.e., provide the rewrite-specific candidate
list) or stop overriding the fallback (keep original.fallbackCandidates) so the
seed remains consistent—ensure the change is made where new
CategorySemanticProfileSeed(...) is invoked and touches the fallbackCandidates
argument to align with ActionIntent.REWRITE.
Comment on lines
+103
to
+105
| PromptSpec spec = factory.createFromConfirmedAxes(axes, "hello", null); | ||
| assertThat(spec).isNotNull(); | ||
| assertThat(spec.getObjective()).isEqualTo(PromptObjective.FACTUAL); |
There was a problem hiding this comment.
성공 케이스가 액션 경로를 실제로 검증하지 않습니다.
objectiveResolver가 항상 FACTUAL을 반환하도록 stub 되어 있어서, createFromConfirmedAxes(...)가 axes.actionType()를 무시해도 이 검증은 통과합니다. 이 PR의 회귀 포인트는 action resolution이므로 spec.getActionType()까지 확인해 두는 편이 안전합니다.
✅ 제안 수정
PromptSpec spec = factory.createFromConfirmedAxes(axes, "hello", null);
assertThat(spec).isNotNull();
+ assertThat(spec.getActionType()).isEqualTo(WritingActionType.ARTICLE_WRITING);
assertThat(spec.getObjective()).isEqualTo(PromptObjective.FACTUAL);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sharedPrompts/src/test/java/org/example/sharedprompts/domain/prompt/domain/service/spec/PromptSpecFactoryTest.java`
around lines 103 - 105, The test currently doesn't verify action resolution
because objectiveResolver is stubbed to always return FACTUAL; update
PromptSpecFactoryTest so that after creating the spec via
factory.createFromConfirmedAxes(axes, "hello", null) you also assert
spec.getActionType() equals the expected action (e.g., axes.actionType() or the
concrete ActionType you set up in the test), ensuring createFromConfirmedAxes
actually uses the axes.actionType() rather than only relying on
objectiveResolver returning FACTUAL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed:
Why:
Impact:
Constraints:
Summary by CodeRabbit
릴리스 노트
버그 수정
리팩토링