feat(strategy): Add minimal strategy layer with decision → goal decomposition - #16
Open
divo12 wants to merge 5 commits into
Open
feat(strategy): Add minimal strategy layer with decision → goal decomposition#16divo12 wants to merge 5 commits into
divo12 wants to merge 5 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
14 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/strategy/parser.ts">
<violation number="1" location="src/strategy/parser.ts:14">
P2: Descriptions containing astral Unicode characters are rejected below the documented 500-character limit because `String.length` counts UTF-16 code units. Count Unicode code points for this limit.</violation>
</file>
<file name="src/strategy/types.ts">
<violation number="1" location="src/strategy/types.ts:13">
P2: After a save→load round trip through FileStrategyStore, `decision.createdAt` and `goal.createdAt`/`updatedAt` come back as ISO strings, not `Date`, because the store writes with `JSON.stringify` and reads with `JSON.parse(...) as Decision[]` without reviving dates. Any caller that trusts the `Date` type and calls `.getTime()`/`.toISOString()` (e.g. goal decay or ordering by `createdAt`) will throw or misbehave. Either deserialize the date fields in the store when reading, or declare these fields as ISO `string` so the persisted shape matches the type contract.</violation>
</file>
<file name="src/cli/runners.ts">
<violation number="1" location="src/cli/runners.ts:452">
P2: `stratiki strategy list` reads each decision's goals but prints only their count, so users cannot inspect the decomposed goals promised by this command. Print each goal's description, rank, and grounding in this branch.</violation>
<violation number="2" location="src/cli/runners.ts:464">
P3: A seed description longer than 500 characters makes parseDecisionSeed throw (src/strategy/parser.ts enforces a 500-char limit and throws), but runStrategyCommand has no try/catch and parseStrategyCommand places no length validation on the seed argument. The resulting uncaught rejection is only surfaced through the generic crash guard, so the user gets a raw surfaced error instead of a usage message for valid-looking CLI input. Validate the description length in parseStrategyCommand (or wrap decompose/save in try/catch) so the long-input case returns a friendly error and non-zero exit code.</violation>
</file>
<file name="src/strategy/store.ts">
<violation number="1" location="src/strategy/store.ts:25">
P1: Concurrent `strategy seed` processes can lose decisions: each reads the same array, appends locally, and the last `writeFile` overwrites the other. Serialize these updates with a lock or use an atomic datastore.</violation>
<violation number="2" location="src/strategy/store.ts:62">
P2: After persistence, `listDecisions()` and `getGoalsForDecision()` return ISO strings where the `Decision` and `Goal` contracts require `Date` objects. Rehydrate decision and goal timestamp fields when loading JSON.</violation>
<violation number="3" location="src/strategy/store.ts:62">
P2: After a save/load round trip, `createdAt` (and `updatedAt` for goals) are no longer `Date` objects. `JSON.stringify(new Date())` emits an ISO string and `JSON.parse` returns a plain string, but `JSON.parse(content) as Decision[]` asserts them back to `Date`. Any consumer calling `.getTime()`, `.toISOString()`, or comparing dates on values returned by `listDecisions()`/`getGoalsForDecision()` will silently misbehave. This contradicts the PR's claim of type-safe JSON handling. Revive the dates after parsing (or store timestamps as strings and adjust the types accordingly).</violation>
<violation number="4" location="src/strategy/store.ts:63">
P1: When a persisted JSON file is malformed, the read methods return `[]` and the next save silently overwrites the file, discarding all existing decisions or goals. Return an empty array only for `ENOENT` and rethrow parse or filesystem errors.</violation>
</file>
<file name="src/cli/commands.ts">
<violation number="1" location="src/cli/commands.ts:856">
P3: The `list` action silently ignores any trailing arguments. `stratiki strategy list unexpected` parses successfully and lists decisions, unlike sibling commands (e.g. `parseBookCommand` returns `Unexpected argument for book ...` and `parseRunCommand` rejects unknown options). Add a guard that errors when `argv.length > 1` for the list action, or route any positional leftover to an error like the other parsers.</violation>
</file>
<file name="test/strategy/decomposer.test.ts">
<violation number="1" location="test/strategy/decomposer.test.ts:67">
P1: Test 1 fails and the grounding/ranking tests are vacuous: decomposeDecision issues a single FTS5 AND query over the whole concatenated description, so no wiki page matches and every goal.groundedIn is empty. The assertion `expect(goalsWithGrounding.length).toBeGreaterThan(0)` can never hold, while tests 2 and 3 assert `0 >= 0` / `totalGrounding === 0` and pass trivially, meaning the grounding and ranking behavior is not actually tested. Search each goal's own text (split the description) instead of the full description, and assert on real grounded entries.</violation>
<violation number="2" location="test/strategy/decomposer.test.ts:101">
P2: The `if (authGoal && coffeeGoal)` guard turns the assertions into a no-op whenever either goal is missing, so a regression that stops extracting one of the two goals (e.g., `parseGoalsFromDescription` failing to split on the period boundary) would silently pass this test. Guarding a test's only assertions hides failures instead of surfacing them — replace it with an explicit `expect(authGoal).toBeDefined()` and `expect(coffeeGoal).toBeDefined()` so the test fails loudly when the expected goals are not produced.</violation>
<violation number="3" location="test/strategy/decomposer.test.ts:102">
P2: The test "ranks goals with better grounding higher" never asserts on the `rank` field, so it does not verify ranking — it only compares `groundedIn.length`. The name promises ranking behavior, but the single structured assertion checks grounding counts, and the enclosing `if (authGoal && coffeeGoal)` guard silently skips even that if either goal isn't found. Assert that both goals exist and assert rank ordering directly.</violation>
</file>
<file name="src/strategy/decomposer.ts">
<violation number="1" location="src/strategy/decomposer.ts:17">
P2: For a multi-sentence decision, this passes every term to FTS5 as one space-separated query. `ContextIndex.search` treats those terms as AND, so pages matching individual goals are excluded; search each goal separately or construct an OR query before assigning grounding.</violation>
<violation number="2" location="src/strategy/decomposer.ts:97">
P2: When a wiki match is found only through its title, this marks the goal ungrounded because it tokenizes `entry.excerpt` only. Include `entry.title` in the token set before checking word overlap.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const { mkdir, writeFile } = await import("node:fs/promises"); | ||
| await mkdir(this.strategyDir, { recursive: true }); | ||
|
|
||
| const decisions = await this.listDecisions(); |
There was a problem hiding this comment.
P1: Concurrent strategy seed processes can lose decisions: each reads the same array, appends locally, and the last writeFile overwrites the other. Serialize these updates with a lock or use an atomic datastore.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/store.ts, line 25:
<comment>Concurrent `strategy seed` processes can lose decisions: each reads the same array, appends locally, and the last `writeFile` overwrites the other. Serialize these updates with a lock or use an atomic datastore.</comment>
<file context>
@@ -0,0 +1,80 @@
+ const { mkdir, writeFile } = await import("node:fs/promises");
+ await mkdir(this.strategyDir, { recursive: true });
+
+ const decisions = await this.listDecisions();
+ decisions.push(decision);
+
</file context>
| "utf8", | ||
| ); | ||
| return JSON.parse(content) as Decision[]; | ||
| } catch { |
There was a problem hiding this comment.
P1: When a persisted JSON file is malformed, the read methods return [] and the next save silently overwrites the file, discarding all existing decisions or goals. Return an empty array only for ENOENT and rethrow parse or filesystem errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/store.ts, line 63:
<comment>When a persisted JSON file is malformed, the read methods return `[]` and the next save silently overwrites the file, discarding all existing decisions or goals. Return an empty array only for `ENOENT` and rethrow parse or filesystem errors.</comment>
<file context>
@@ -0,0 +1,80 @@
+ "utf8",
+ );
+ return JSON.parse(content) as Decision[];
+ } catch {
+ return [];
+ }
</file context>
| const goalsWithGrounding = result.goals.filter( | ||
| (g) => g.groundedIn.length > 0, | ||
| ); | ||
| expect(goalsWithGrounding.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
P1: Test 1 fails and the grounding/ranking tests are vacuous: decomposeDecision issues a single FTS5 AND query over the whole concatenated description, so no wiki page matches and every goal.groundedIn is empty. The assertion expect(goalsWithGrounding.length).toBeGreaterThan(0) can never hold, while tests 2 and 3 assert 0 >= 0 / totalGrounding === 0 and pass trivially, meaning the grounding and ranking behavior is not actually tested. Search each goal's own text (split the description) instead of the full description, and assert on real grounded entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/strategy/decomposer.test.ts, line 67:
<comment>Test 1 fails and the grounding/ranking tests are vacuous: decomposeDecision issues a single FTS5 AND query over the whole concatenated description, so no wiki page matches and every goal.groundedIn is empty. The assertion `expect(goalsWithGrounding.length).toBeGreaterThan(0)` can never hold, while tests 2 and 3 assert `0 >= 0` / `totalGrounding === 0` and pass trivially, meaning the grounding and ranking behavior is not actually tested. Search each goal's own text (split the description) instead of the full description, and assert on real grounded entries.</comment>
<file context>
@@ -0,0 +1,139 @@
+ const goalsWithGrounding = result.goals.filter(
+ (g) => g.groundedIn.length > 0,
+ );
+ expect(goalsWithGrounding.length).toBeGreaterThan(0);
+ } finally {
+ index.close();
</file context>
| throw new Error("Decision description cannot be empty"); | ||
| } | ||
|
|
||
| if (description.length > 500) { |
There was a problem hiding this comment.
P2: Descriptions containing astral Unicode characters are rejected below the documented 500-character limit because String.length counts UTF-16 code units. Count Unicode code points for this limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/parser.ts, line 14:
<comment>Descriptions containing astral Unicode characters are rejected below the documented 500-character limit because `String.length` counts UTF-16 code units. Count Unicode code points for this limit.</comment>
<file context>
@@ -0,0 +1,24 @@
+ throw new Error("Decision description cannot be empty");
+ }
+
+ if (description.length > 500) {
+ throw new Error("Decision description must be 500 characters or less");
+ }
</file context>
Suggested change
| if (description.length > 500) { | |
| if ([...description].length > 500) { |
| export interface Decision { | ||
| readonly id: string; | ||
| readonly description: string; | ||
| readonly createdAt: Date; |
There was a problem hiding this comment.
P2: After a save→load round trip through FileStrategyStore, decision.createdAt and goal.createdAt/updatedAt come back as ISO strings, not Date, because the store writes with JSON.stringify and reads with JSON.parse(...) as Decision[] without reviving dates. Any caller that trusts the Date type and calls .getTime()/.toISOString() (e.g. goal decay or ordering by createdAt) will throw or misbehave. Either deserialize the date fields in the store when reading, or declare these fields as ISO string so the persisted shape matches the type contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/types.ts, line 13:
<comment>After a save→load round trip through FileStrategyStore, `decision.createdAt` and `goal.createdAt`/`updatedAt` come back as ISO strings, not `Date`, because the store writes with `JSON.stringify` and reads with `JSON.parse(...) as Decision[]` without reviving dates. Any caller that trusts the `Date` type and calls `.getTime()`/`.toISOString()` (e.g. goal decay or ordering by `createdAt`) will throw or misbehave. Either deserialize the date fields in the store when reading, or declare these fields as ISO `string` so the persisted shape matches the type contract.</comment>
<file context>
@@ -0,0 +1,39 @@
+export interface Decision {
+ readonly id: string;
+ readonly description: string;
+ readonly createdAt: Date;
+ readonly status: DecisionStatus;
+}
</file context>
| return contextEntries | ||
| .filter((entry) => { | ||
| const entryWords = new Set( | ||
| entry.excerpt |
There was a problem hiding this comment.
P2: When a wiki match is found only through its title, this marks the goal ungrounded because it tokenizes entry.excerpt only. Include entry.title in the token set before checking word overlap.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/decomposer.ts, line 97:
<comment>When a wiki match is found only through its title, this marks the goal ungrounded because it tokenizes `entry.excerpt` only. Include `entry.title` in the token set before checking word overlap.</comment>
<file context>
@@ -0,0 +1,122 @@
+ return contextEntries
+ .filter((entry) => {
+ const entryWords = new Set(
+ entry.excerpt
+ .toLowerCase()
+ .split(/\W+/u)
</file context>
Suggested change
| entry.excerpt | |
| `${entry.title} ${entry.excerpt}` |
| decision: Decision, | ||
| bookIndex: ContextIndex, | ||
| ): DecompositionResult { | ||
| const contextEntries = bookIndex.search(decision.description, 10); |
There was a problem hiding this comment.
P2: For a multi-sentence decision, this passes every term to FTS5 as one space-separated query. ContextIndex.search treats those terms as AND, so pages matching individual goals are excluded; search each goal separately or construct an OR query before assigning grounding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/decomposer.ts, line 17:
<comment>For a multi-sentence decision, this passes every term to FTS5 as one space-separated query. `ContextIndex.search` treats those terms as AND, so pages matching individual goals are excluded; search each goal separately or construct an OR query before assigning grounding.</comment>
<file context>
@@ -0,0 +1,122 @@
+ decision: Decision,
+ bookIndex: ContextIndex,
+): DecompositionResult {
+ const contextEntries = bookIndex.search(decision.description, 10);
+ const goals = extractGoalsFromDecision(decision, contextEntries);
+
</file context>
| `${this.strategyDir}/decisions.json`, | ||
| "utf8", | ||
| ); | ||
| return JSON.parse(content) as Decision[]; |
There was a problem hiding this comment.
P2: After a save/load round trip, createdAt (and updatedAt for goals) are no longer Date objects. JSON.stringify(new Date()) emits an ISO string and JSON.parse returns a plain string, but JSON.parse(content) as Decision[] asserts them back to Date. Any consumer calling .getTime(), .toISOString(), or comparing dates on values returned by listDecisions()/getGoalsForDecision() will silently misbehave. This contradicts the PR's claim of type-safe JSON handling. Revive the dates after parsing (or store timestamps as strings and adjust the types accordingly).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/strategy/store.ts, line 62:
<comment>After a save/load round trip, `createdAt` (and `updatedAt` for goals) are no longer `Date` objects. `JSON.stringify(new Date())` emits an ISO string and `JSON.parse` returns a plain string, but `JSON.parse(content) as Decision[]` asserts them back to `Date`. Any consumer calling `.getTime()`, `.toISOString()`, or comparing dates on values returned by `listDecisions()`/`getGoalsForDecision()` will silently misbehave. This contradicts the PR's claim of type-safe JSON handling. Revive the dates after parsing (or store timestamps as strings and adjust the types accordingly).</comment>
<file context>
@@ -0,0 +1,80 @@
+ `${this.strategyDir}/decisions.json`,
+ "utf8",
+ );
+ return JSON.parse(content) as Decision[];
+ } catch {
+ return [];
</file context>
| }; | ||
| } | ||
|
|
||
| if (action === "list") { |
There was a problem hiding this comment.
P3: The list action silently ignores any trailing arguments. stratiki strategy list unexpected parses successfully and lists decisions, unlike sibling commands (e.g. parseBookCommand returns Unexpected argument for book ... and parseRunCommand rejects unknown options). Add a guard that errors when argv.length > 1 for the list action, or route any positional leftover to an error like the other parsers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/commands.ts, line 856:
<comment>The `list` action silently ignores any trailing arguments. `stratiki strategy list unexpected` parses successfully and lists decisions, unlike sibling commands (e.g. `parseBookCommand` returns `Unexpected argument for book ...` and `parseRunCommand` rejects unknown options). Add a guard that errors when `argv.length > 1` for the list action, or route any positional leftover to an error like the other parsers.</comment>
<file context>
@@ -832,6 +842,34 @@ function parseBookCommand(argv: string[]): CliCommand {
+ };
+ }
+
+ if (action === "list") {
+ return { action: "list", description: null, exitCode: 0, kind: "strategy" };
+ }
</file context>
| return; | ||
| } | ||
|
|
||
| const decision = parseDecisionSeed({ description: command.description }); |
There was a problem hiding this comment.
P3: A seed description longer than 500 characters makes parseDecisionSeed throw (src/strategy/parser.ts enforces a 500-char limit and throws), but runStrategyCommand has no try/catch and parseStrategyCommand places no length validation on the seed argument. The resulting uncaught rejection is only surfaced through the generic crash guard, so the user gets a raw surfaced error instead of a usage message for valid-looking CLI input. Validate the description length in parseStrategyCommand (or wrap decompose/save in try/catch) so the long-input case returns a friendly error and non-zero exit code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/runners.ts, line 464:
<comment>A seed description longer than 500 characters makes parseDecisionSeed throw (src/strategy/parser.ts enforces a 500-char limit and throws), but runStrategyCommand has no try/catch and parseStrategyCommand places no length validation on the seed argument. The resulting uncaught rejection is only surfaced through the generic crash guard, so the user gets a raw surfaced error instead of a usage message for valid-looking CLI input. Validate the description length in parseStrategyCommand (or wrap decompose/save in try/catch) so the long-input case returns a friendly error and non-zero exit code.</comment>
<file context>
@@ -421,6 +421,69 @@ export async function runBookCommand(
+ return;
+ }
+
+ const decision = parseDecisionSeed({ description: command.description });
+ const index = await ContextIndex.buildFromDirectory(bookDir);
+ try {
</file context>
- Add strategy module with Decision and Goal types - Implement decision seed parser with validation - Add goal decomposer that grounds decisions in book context - Add file-based strategy store under .strategy/ - Add CLI commands: stratiki strategy seed|list - Add tests for parser and decomposer with fixture wikis - Goals are ranked by grounding quality from existing knowledge Co-authored-by: divo12 <divo12@users.noreply.github.com>
- Add 'as Decision[]' and 'as Goal[]' type assertions - Fix lint errors in runners and tests - Create immutable copy for sort operations Co-authored-by: divo12 <divo12@users.noreply.github.com>
….strategy - Use openWikiStrategyDir from openwiki-home helpers - Reuse existing company-mode home infrastructure - Add strategy dir to ensureOpenWikiHome initialization - Update store documentation to reflect correct location Co-authored-by: divo12 <divo12@users.noreply.github.com>
Co-authored-by: divo12 <divo12@users.noreply.github.com>
- Add getStratikiStrategyDir() lazy getter - Update runStrategyCommand to use getStratikiStrategyDir() for storage - Update runStrategyCommand to use getStratikiCompanyWikiDir() for grounding - Call ensureStratikiHome() before saving decisions - Update strategy dir creation in ensureStratikiHome() - Remove unused openWikiStrategyDir constant - Update store.ts documentation to specify ~/.stratiki/strategy This ensures strategy decisions are stored in ~/.stratiki/strategy and grounded against the company brain wiki at ~/.stratiki/wiki. Co-authored-by: divo12 <divo12@users.noreply.github.com>
cursor
Bot
force-pushed
the
cursor/strategy-stacked-6abe
branch
from
September 1, 2026 07:19
f86c9e2 to
44cb495
Compare
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
2 participants
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.
Strategy Layer
Stacked on #15 — This PR adds the strategy layer on top of the company brain.
Changes
stratiki strategy seed <description>parses a decision and decomposes it into ranked goalsstratiki strategy listshows all seeded decisions and their goals~/.stratiki/strategy/(correct path from the start)~/.stratiki/wiki/Implementation
strategy/parser.ts): Parses decision seed inputstrategy/decomposer.ts): Decomposes decisions into goals using the company wiki contextstrategy/store.ts): Persists decisions and goals as JSON filesstrategy/types.ts): Decision and Goal interfacesStorage Path
Storage location is
~/.stratiki/strategy/viagetStratikiStrategyDir():ensureStratikiHome()~/.stratiki/wiki/What's NOT Included
Per requirements:
Testing
This is a focused PR that adds the minimal strategy layer without scheduler or runtime complexity.
Summary by cubic
Adds a minimal strategy layer that turns a seeded decision into ranked goals grounded in the company wiki, exposed via two new CLI commands:
stratiki strategy seed <description>andstratiki strategy list.New Features
~/.stratiki/strategy/, created lazily byensureStratikiHome().Written for commit 44cb495. Summary will update on new commits.