diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,39 @@ # Changelog +## 0.4.0.0 — 2026-08-17 + +### Changed + +- The embedding dimension-mismatch diagnostic names `kioku.memories.embedding` rather than + `kiroku.kioku_memories.embedding`, following the projection relocation in + `kioku/0012-relocate-projections-to-kioku-schema`. + + +### Added + +- `KIOKU_MEMORY_SPACE` (default `kioku_legacy`) and `KIOKU_ACTOR` (default `kioku_cli`) decide the + memory space CLI commands write into and the principal writes are attributed to. A malformed + value is a startup error rather than a silent fallback. The worker is not pinned to one space: + it acts in whichever space a claimed timer names. + +### Changed + +- `KIOKU_MEMORY_SPACE` now also decides what commands *read*. `kioku recall`, `kioku scenes`, and + `kioku persona` return nothing outside it. +- **Breaking:** `kioku recall --scope NAMESPACE` no longer parses. A bare namespace meant *every + scope in the namespace* to `recall` and *the global bucket* to `kioku scenes` — one spelling, + two answers. Each target now has its own flag, exactly one of which is required: + `--scope NAMESPACE:KIND:REF` for one entity scope, `--global-bucket NAMESPACE` for the rows with + no entity scope, and `--namespace-wide NAMESPACE` for every scope under it. `--namespace-wide + mori` is what `--scope mori` returned; `--scope mori:repo:web` is unchanged. It is an error + rather than a silent re-reading because the two readings differ in how many rows come back, and + a script would otherwise keep exiting zero while returning a fraction of them. +- `kioku recall` can ask for the exact global bucket for the first time. +- `kioku recall` prints the target and memory space it searched to stderr. stdout is byte-for-byte + unchanged, so a script piping hits is unaffected. +- `kioku scenes` and `kioku persona` are untouched: `--scope NAMESPACE` still means the global + bucket there, because neither has a namespace-wide reading to be confused with. + ## 0.3.0.0 — 2026-08-05 ### Changed diff --git a/kioku-cli.cabal b/kioku-cli.cabal --- a/kioku-cli.cabal +++ b/kioku-cli.cabal @@ -1,6 +1,6 @@ cabal-version: 3.0 name: kioku-cli -version: 0.3.0.0 +version: 0.4.0.0 synopsis: kioku command-line interface description: Command-line entry point for kioku demos and operational commands. @@ -26,6 +26,7 @@ ghc-options: -Wall -Wcompat -Widentities -Wincomplete-record-updates -Wincomplete-uni-patterns -Wpartial-fields -Wredundant-constraints + -Werror=incomplete-patterns common shared default-language: GHC2024 @@ -41,10 +42,11 @@ TemplateHaskell library - import: warnings, shared - hs-source-dirs: src + import: warnings, shared + hs-source-dirs: src exposed-modules: Kioku.Cli + Kioku.Cli.Commands.Artifacts Kioku.Cli.Commands.Demo Kioku.Cli.Commands.DemoSession Kioku.Cli.Commands.Distill @@ -52,17 +54,31 @@ Kioku.Cli.Commands.Recall Kioku.Cli.Commands.Scenes Kioku.Cli.Commands.Worker + Kioku.Cli.Context Kioku.Cli.Options Kioku.Cli.Scope + -- Same GHC 9.12.4 coercion-optimiser bug that kioku-core works around; here it + -- fells the profiled build of Kioku.Cli.Commands.DemoSession: + -- + -- panic! (the 'impossible' happened) / coercionKind / ConsSymbolDef + -- pprPanic, called at compiler/GHC/Core/Coercion.hs:2550:17 + -- + -- The type-level Symbol axioms arrive transitively through kioku-core's + -- KindID-typed identifiers, so this package needs the same opt-out even though + -- it never mentions mmzk-typeid itself. See kioku-core.cabal for the full + -- write-up. Profiled way only; ordinary builds keep the coercion optimiser. + -- mori://MMZK1526/mmzk-typeid/upstream-issues/mmzk-typeid-kindid-ghc-9-12-4-profiling-coercionkind-panic + ghc-prof-options: -fno-opt-coercion build-depends: , async >=2.2 && <2.3 , base >=4.21 && <5 , containers >=0.6 && <0.8 + , directory >=1.3 && <1.4 , effectful >=2.5 && <2.7 - , kioku-api ^>=0.3.0.0 - , kioku-core ^>=0.3.0.0 - , kiroku-store ^>=0.3.0.1 + , kioku-api ^>=0.4.0.0 + , kioku-core ^>=0.4.0.0 + , kiroku-store ^>=0.8.0.0 , optparse-applicative >=0.18 && <0.20 , text >=2.1 && <2.2 , time >=1.12 && <1.15 @@ -74,21 +90,34 @@ ghc-options: -threaded -rtsopts -with-rtsopts=-N build-depends: , base >=4.21 && <5 - , kioku-cli ^>=0.3.0.0 + , kioku-cli ^>=0.4.0.0 test-suite kioku-cli-test - import: warnings, shared - type: exitcode-stdio-1.0 - main-is: Main.hs - hs-source-dirs: test - other-modules: Kioku.Cli.ParserSpec - ghc-options: -threaded -rtsopts -with-rtsopts=-N + import: warnings, shared + type: exitcode-stdio-1.0 + main-is: Main.hs + hs-source-dirs: test + other-modules: + Kioku.Cli.ParserSpec + Kioku.Cli.RecallEndToEndSpec + + ghc-options: -threaded -rtsopts -with-rtsopts=-N + + -- Kioku.Cli.RecallEndToEndSpec runs the real binary as a subprocess: stdout, stderr and the + -- environment are process-wide, and tasty runs cases concurrently, so redirecting them in + -- process would race the rest of the suite. + build-tool-depends: kioku-cli:kioku build-depends: - , base >=4.21 && <5 - , kioku-api ^>=0.3.0.0 - , kioku-cli ^>=0.3.0.0 - , kioku-core ^>=0.3.0.0 - , optparse-applicative >=0.18 - , tasty >=1.5 - , tasty-hunit >=0.10 - , text >=2.1 + , base >=4.21 && <5 + , effectful >=2.5 && <2.7 + , kioku-api ^>=0.4.0.0 + , kioku-cli ^>=0.4.0.0 + , kioku-core ^>=0.4.0.0 + , kioku-migrations:test-support ^>=0.4.0.0 + , kiroku-store ^>=0.8.0.0 + , optparse-applicative >=0.18 + , process >=1.6 && <1.7 + , tasty >=1.5 + , tasty-hunit >=0.10 + , text >=2.1 + , time >=1.12 && <1.15 diff --git a/src/Kioku/Cli.hs b/src/Kioku/Cli.hs --- a/src/Kioku/Cli.hs +++ b/src/Kioku/Cli.hs @@ -3,6 +3,7 @@ ) where +import Kioku.Cli.Commands.Artifacts (ArtifactsOptions, artifactsOptionsParser, runArtifacts) import Kioku.Cli.Commands.Demo (DemoOptions, demoOptionsParser, runDemo) import Kioku.Cli.Commands.DemoSession (DemoSessionOptions, demoSessionOptionsParser, runDemoSession) import Kioku.Cli.Commands.Distill (DistillOptions, distillOptionsParser, runDistill) @@ -12,7 +13,7 @@ import Kioku.Cli.Commands.Worker (WorkerOptions, runWorker, workerOptionsParser) import Options.Applicative -data Command = Demo DemoOptions | DemoSession DemoSessionOptions | Distill DistillOptions | Persona PersonaOptions | Recall RecallOptions | Scenes ScenesOptions | Worker WorkerOptions +data Command = Artifacts ArtifactsOptions | Demo DemoOptions | DemoSession DemoSessionOptions | Distill DistillOptions | Persona PersonaOptions | Recall RecallOptions | Scenes ScenesOptions | Worker WorkerOptions main :: IO () main = run =<< execParser opts @@ -38,6 +39,12 @@ "distill" (info (Distill <$> (helper <*> distillOptionsParser)) (progDesc "Run distillation commands")) <> command + "migrate-artifacts" + ( info + (Artifacts <$> (helper <*> artifactsOptionsParser)) + (progDesc "Move pre-partition .kioku scene and persona mirrors into a memory space") + ) + <> command "persona" (info (Persona <$> (helper <*> personaOptionsParser)) (progDesc "Print distilled L3 persona")) <> command @@ -51,6 +58,7 @@ (info (Worker <$> (helper <*> workerOptionsParser)) (progDesc "Run kioku background workers")) run :: Command -> IO () +run (Artifacts opts) = runArtifacts opts run (Demo opts) = runDemo opts run (DemoSession opts) = runDemoSession opts run (Distill opts) = runDistill opts diff --git a/src/Kioku/Cli/Commands/Artifacts.hs b/src/Kioku/Cli/Commands/Artifacts.hs new file mode 100644 --- /dev/null +++ b/src/Kioku/Cli/Commands/Artifacts.hs @@ -0,0 +1,98 @@ +-- | @kioku migrate-artifacts@: move the pre-partition workspace mirrors into a memory space. +-- +-- Before memory spaces existed, scene and persona mirrors were written to @.kioku\/scenes@ and +-- @.kioku\/persona@, keyed by scope alone. Two spaces holding the same scope would have written +-- to the same file, so the layout is now @.kioku\/spaces\/\\/{scenes,persona}@ and +-- nothing writes to the old tree any more. This command relocates what is already there. +-- +-- It is a dry run unless @--apply@ is passed. That default is the point: the command exists so +-- an operator can read exactly which file would land where, and see any collision, before +-- anything is written. +module Kioku.Cli.Commands.Artifacts + ( ArtifactsOptions (..), + artifactsOptionsParser, + runArtifacts, + ) +where + +import Control.Monad (when) +import Data.Text qualified as Text +import Kioku.Api.Access (memorySpaceIdText) +import Kioku.Cli.Context (cliMemorySpace) +import Kioku.Workspace + ( ArtifactMove (..), + MoveVerdict (..), + applyArtifactMigration, + planArtifactMigration, + ) +import Options.Applicative +import System.Directory (getCurrentDirectory) +import System.Exit (ExitCode (..), exitWith) + +data ArtifactsOptions = ArtifactsOptions + { workspace :: !(Maybe FilePath), + apply :: !Bool + } + deriving stock (Eq, Show) + +artifactsOptionsParser :: Parser ArtifactsOptions +artifactsOptionsParser = + ArtifactsOptions + <$> optional + ( strOption + ( long "workspace" + <> metavar "DIR" + <> help "Workspace holding .kioku (default: the current directory)" + ) + ) + <*> switch + ( long "apply" + <> help "Copy the files (default: report what would happen and write nothing)" + ) + +-- | The destination space comes from @KIOKU_MEMORY_SPACE@, which defaults to @kioku_legacy@. +-- +-- That default is the same rule the database backfill follows: every artifact in the historical +-- tree was written before the partition existed, so it belongs to the one explicit legacy space +-- unless the operator says otherwise. See @docs\/adr\/legacy-data-lands-in-one-explicit-space.md@. +runArtifacts :: ArtifactsOptions -> IO () +runArtifacts opts = do + space <- cliMemorySpace + workspace <- maybe getCurrentDirectory pure opts.workspace + moves <- planArtifactMigration workspace space + putStrLn + ( "kioku artifact migration (" + <> (if opts.apply then "apply" else "dry run") + <> ") for memory space " + <> Text.unpack (memorySpaceIdText space) + ) + if null moves + then putStrLn " (no pre-partition scene or persona mirrors found)" + else mapM_ (putStrLn . renderMove) moves + when opts.apply (applyArtifactMigration moves) + putStrLn (summarize moves) + -- A collision is a refusal, and a refusal a script cannot see is not a refusal. It is + -- reported in dry-run mode too, because the whole purpose of the dry run is to find out + -- before applying. + when (any ((== MoveCollision) . (.verdict)) moves) (exitWith (ExitFailure 1)) + +renderMove :: ArtifactMove -> String +renderMove move = + " " <> verdictLabel move.verdict <> " " <> move.source <> " -> " <> move.destination + +verdictLabel :: MoveVerdict -> String +verdictLabel = \case + MoveReady -> "copy " + MoveAlreadyMigrated -> "migrated " + MoveCollision -> "COLLISION" + +summarize :: [ArtifactMove] -> String +summarize moves = + show (count MoveReady) + <> " to copy, " + <> show (count MoveAlreadyMigrated) + <> " already migrated, " + <> show (count MoveCollision) + <> " refused as collisions." + where + count verdict = length (filter ((== verdict) . (.verdict)) moves) diff --git a/src/Kioku/Cli/Commands/Demo.hs b/src/Kioku/Cli/Commands/Demo.hs --- a/src/Kioku/Cli/Commands/Demo.hs +++ b/src/Kioku/Cli/Commands/Demo.hs @@ -9,9 +9,11 @@ import Data.Set qualified as Set import Data.Text qualified as Text import Data.Time (getCurrentTime) +import Kioku.Api.Access (memoryContextRecordedActor, memoryContextSpace) import Kioku.Api.Scope (MemoryScope (..), Namespace (..), ScopeKind (..)) import Kioku.Api.Types (Confidence (..), MemoryRecord (..), MemoryType (..)) import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliMemoryContext) import Kioku.Cli.Options (redactConnectionString, yesWriteEventsFlag) import Kioku.Id (genMemoryId, idText) import Kioku.Memory qualified as Memory @@ -42,6 +44,7 @@ putStrLn "kioku demo appends permanent memory events (kioku has no delete)." putStrLn ("Target: " <> Text.unpack (redactConnectionString (Text.pack connStr))) putStrLn "Scope: kioku_demo/demo/demo" + context <- cliMemoryContext withNoopAppEnv (defaultConnectionSettings (Text.pack connStr)) \env -> do mid <- genMemoryId now <- getCurrentTime @@ -49,6 +52,9 @@ payload = RecordMemoryData { memoryId = mid, + memorySpaceId = memoryContextSpace context, + actorPrincipal = memoryContextRecordedActor context, + ownerPrincipal = Nothing, agentId = "demo-agent", sessionId = Nothing, scope = scope, @@ -61,8 +67,8 @@ recordedAt = now } result <- runAppIO env do - writeResult <- Memory.record payload - recallResult <- Recall.getActiveByScope scope + writeResult <- Memory.recordWithContext context payload + recallResult <- Recall.getActiveByScope (memoryContextSpace context) scope pure (writeResult, recallResult) case result of Left storeErr -> ioError (userError ("kioku demo store error: " <> show storeErr)) diff --git a/src/Kioku/Cli/Commands/DemoSession.hs b/src/Kioku/Cli/Commands/DemoSession.hs --- a/src/Kioku/Cli/Commands/DemoSession.hs +++ b/src/Kioku/Cli/Commands/DemoSession.hs @@ -7,8 +7,10 @@ import Data.Text qualified as Text import Data.Time (getCurrentTime) +import Kioku.Api.Access (memoryContextRecordedActor, memoryContextSpace) import Kioku.App (runAppIO, withNoopAppEnv) import Kioku.Cli.Commands.Demo (demoScope) +import Kioku.Cli.Context (cliMemoryContext) import Kioku.Cli.Options (redactConnectionString, yesWriteEventsFlag) import Kioku.Id (genSessionId, idText) import Kioku.Session qualified as Session @@ -31,13 +33,19 @@ putStrLn ("Target: " <> Text.unpack (redactConnectionString (Text.pack connStr))) putStrLn "Scope: kioku_demo/demo/demo" putStrLn "Note: completing this session schedules a distillation timer; a running worker will process it (an LLM call)." + context <- cliMemoryContext withNoopAppEnv (defaultConnectionSettings (Text.pack connStr)) \env -> do sid <- genSessionId now <- getCurrentTime let scope = demoScope + space = memoryContextSpace context + actor = memoryContextRecordedActor context startPayload = StartSessionData { sessionId = sid, + memorySpaceId = space, + actorPrincipal = actor, + ownerPrincipal = Nothing, agentId = "demo-agent", focus = "demo", scope = scope, @@ -50,6 +58,8 @@ turnPayload = RecordTurnData { sessionId = sid, + memorySpaceId = space, + actorPrincipal = actor, turnId = idText sid <> "-turn-1", turnIndex = 1, role = "user", @@ -62,16 +72,18 @@ completePayload = CompleteSessionData { sessionId = sid, + memorySpaceId = space, + actorPrincipal = actor, completedAt = now, modelUsed = Just "demo-model", summary = Just "Demo session completed" } result <- runAppIO env do - startResult <- Session.start startPayload - turnResult <- Session.recordTurn turnPayload - completeResult <- Session.complete completePayload - rowResult <- Session.getById sid - turnsResult <- Session.getTurns sid + startResult <- Session.startWithContext context startPayload + turnResult <- Session.recordTurnWithContext context turnPayload + completeResult <- Session.completeWithContext context completePayload + rowResult <- Session.getById space sid + turnsResult <- Session.getTurns space sid pure (startResult, turnResult, completeResult, rowResult, turnsResult) case result of Left storeErr -> ioError (userError ("kioku session demo store error: " <> show storeErr)) diff --git a/src/Kioku/Cli/Commands/Distill.hs b/src/Kioku/Cli/Commands/Distill.hs --- a/src/Kioku/Cli/Commands/Distill.hs +++ b/src/Kioku/Cli/Commands/Distill.hs @@ -8,6 +8,7 @@ import Data.Text qualified as Text import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliMemoryContext) import Kioku.Cli.Options (boundedIntReader) import Kioku.Distill.L1 (L1Outcome (..), L1RunMode (..), L1Summary (..), distillSessionL1, recallCandidates, scopedScanCandidates) import Kioku.Distill.Runtime (newDistillRuntime) @@ -68,6 +69,7 @@ runDistill opts = do connStr <- requireEnv "PG_CONNECTION_STRING" rt <- newDistillRuntime + context <- cliMemoryContext recallConfig <- case opts.candidateSource of CandidateScan -> pure Nothing @@ -81,7 +83,7 @@ pure (recallCandidates (toEmbeddingModel config) capability opts.candidateLimit) _ -> pure (scopedScanCandidates opts.candidateLimit) - distillSessionL1 (runMode opts) rt finder opts.sessionId + distillSessionL1 context (runMode opts) rt finder opts.sessionId case result of Left storeErr -> ioError (userError ("kioku distill store error: " <> show storeErr)) Right (Left l1Err) -> ioError (userError ("kioku distill error: " <> show l1Err)) diff --git a/src/Kioku/Cli/Commands/Persona.hs b/src/Kioku/Cli/Commands/Persona.hs --- a/src/Kioku/Cli/Commands/Persona.hs +++ b/src/Kioku/Cli/Commands/Persona.hs @@ -8,6 +8,7 @@ import Data.Text qualified as Text import Kioku.Api.Scope (MemoryScope) import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliMemorySpace) import Kioku.Cli.Scope (parseScope) import Kioku.Distill.L3 (PersonaRow (..), getPersonaByScope) import Kiroku.Store.Connection (defaultConnectionSettings) @@ -32,8 +33,9 @@ runPersona :: PersonaOptions -> IO () runPersona opts = do connStr <- requireEnv "PG_CONNECTION_STRING" + space <- cliMemorySpace withNoopAppEnv (defaultConnectionSettings (Text.pack connStr)) \env -> do - result <- runAppIO env (getPersonaByScope opts.scope) + result <- runAppIO env (getPersonaByScope space opts.scope) case result of Left storeErr -> ioError (userError ("kioku persona store error: " <> show storeErr)) Right Nothing -> putStrLn "(no persona yet)" diff --git a/src/Kioku/Cli/Commands/Recall.hs b/src/Kioku/Cli/Commands/Recall.hs --- a/src/Kioku/Cli/Commands/Recall.hs +++ b/src/Kioku/Cli/Commands/Recall.hs @@ -1,28 +1,53 @@ +-- | @kioku recall@: the one command that can be asked to widen what it searches. +-- +-- Every other command takes a @--scope@ that means exactly one scope. Recall used to take the +-- same flag and mean two different things by it: @--scope mori:repo:web@ matched that entity +-- exactly, while @--scope mori@ dropped the scope filter and searched the whole namespace — the +-- opposite of what @kioku scenes --scope mori@ does with the same text. That was the command-line +-- half of the overload "Kioku.Api.Recall" removed from the library. +-- +-- Each target now has exactly one spelling, and none of them is a bare namespace: +-- +-- > kioku recall QUERY --scope NAMESPACE:KIND:REF -- ExactScope (ScopeEntity …) +-- > kioku recall QUERY --global-bucket NAMESPACE -- ExactScope (ScopeGlobal …) +-- > kioku recall QUERY --namespace-wide NAMESPACE -- NamespaceWide … +-- +-- @--scope NAMESPACE@ is a parse error naming the other two. It would have been tidier to let it +-- mean the global bucket, matching @kioku scenes@ — but that silently narrows every existing +-- @kioku recall --scope mori@ to a fraction of its rows, with no compiler to warn and a zero exit +-- status, which is the direction @docs\/adr\/an-explicit-recall-target-replaces-the-overloaded-scope.md@ +-- records as the unsafe one. module Kioku.Cli.Commands.Recall ( RecallOptions (..), recallOptionsParser, + recallTargetParser, + describeTarget, + bareNamespaceScopeError, runRecall, ) where import Data.Text (Text) import Data.Text qualified as Text -import Kioku.Api.Scope (MemoryScope) +import Kioku.Api.Access (memoryContextSpace, memorySpaceIdText) +import Kioku.Api.Scope (MemoryScope (..), Namespace (..), ScopeKind (..)) import Kioku.Api.Types (MemoryRecord (..)) import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliMemoryContext) import Kioku.Cli.Options (boundedIntReader) -import Kioku.Cli.Scope (parseScope) +import Kioku.Cli.Scope (parseNamespaceOnly, parseScope) import Kioku.Memory.Embedding (EmbeddingConfig (..), resolveEmbeddingConfig, toEmbeddingModel) -import Kioku.Recall (RecallHit (..), RecallRequest (..), RecallStrategy (..), recall) +import Kioku.Recall (RecallHit (..), RecallStrategy (..), RecallTarget (..), mkRecallQuery, recall) import Kioku.Recall.Capability (detectVectorCapability) import Kiroku.Store.Connection (defaultConnectionSettings) import Options.Applicative import System.Environment (lookupEnv) +import System.IO (hPutStrLn, stderr) import Text.Printf (printf) data RecallOptions = RecallOptions { query :: !Text, - scope :: !MemoryScope, + target :: !RecallTarget, strategy :: !RecallStrategy, limit :: !Int, showScores :: !Bool @@ -33,12 +58,7 @@ recallOptionsParser = RecallOptions <$> (Text.pack <$> argument str (metavar "QUERY")) - <*> option - (eitherReader parseScope) - ( long "scope" - <> metavar "NAMESPACE[:KIND:REF]" - <> help "Memory scope to search; REF may contain ':'" - ) + <*> recallTargetParser <*> option (eitherReader parseStrategy) ( long "strategy" @@ -58,26 +78,109 @@ <> help "Print fused scores and component ranks" ) +-- | Exactly one of the three target flags, and never two. +-- +-- The mutual exclusion is the same construction @kioku worker@ uses for its one-shot modes: +-- alternatives consume the flag they name, so a second target flag is left over and optparse +-- reports it by name. Omitting all three is @Missing:@ with the three forms listed, which is the +-- help text an operator wants at that moment anyway. +recallTargetParser :: Parser RecallTarget +recallTargetParser = + exactEntity <|> globalBucket <|> namespaceWide + where + exactEntity = + ExactScope + <$> option + (eitherReader parseExactEntityScope) + ( long "scope" + <> metavar "NAMESPACE:KIND:REF" + <> help "Search exactly this entity scope; REF may contain ':'" + ) + + globalBucket = + ExactScope . ScopeGlobal + <$> option + (eitherReader parseNamespaceOnly) + ( long "global-bucket" + <> metavar "NAMESPACE" + <> help "Search only the rows recorded in NAMESPACE with no entity scope" + ) + + namespaceWide = + NamespaceWide + <$> option + (eitherReader parseNamespaceOnly) + ( long "namespace-wide" + <> metavar "NAMESPACE" + <> help "Search every scope in NAMESPACE: the global bucket and every entity under it" + ) + +-- | @--scope@ accepts the shared @NAMESPACE:KIND:REF@ grammar and then refuses the one result +-- that would be ambiguous. +-- +-- Reusing 'parseScope' rather than writing a second grammar is deliberate: the rules about which +-- colons split, and which characters a namespace may hold, must not drift between @kioku recall@ +-- and @kioku scenes@. +parseExactEntityScope :: String -> Either String MemoryScope +parseExactEntityScope raw = do + scope <- parseScope raw + case scope of + ScopeGlobal (Namespace ns) -> Left (bareNamespaceScopeError (Text.unpack ns)) + entity -> Right entity + +-- | What an operator sees when they type the spelling that used to work. +-- +-- It names both replacements and says which one reproduces the old behavior, because the whole +-- reason this is an error rather than a silent re-reading is that the two answers differ in how +-- many rows come back. +bareNamespaceScopeError :: String -> String +bareNamespaceScopeError ns = + unlines + [ "a bare namespace is ambiguous for recall, so --scope will not accept one.", + " --global-bucket " <> ns <> " rows in " <> ns <> " recorded with no entity scope", + " --namespace-wide " <> ns <> " every scope in " <> ns <> " (what --scope " <> ns <> " returned before)" + ] + <> "--scope takes a full NAMESPACE:KIND:REF entity scope." + +-- | The target and space a run actually searched, for the operator rather than for a script. +describeTarget :: RecallTarget -> String +describeTarget = \case + ExactScope (ScopeGlobal (Namespace ns)) -> + "the global bucket of " <> Text.unpack ns + ExactScope (ScopeEntity (Namespace ns) (ScopeKind kind) ref) -> + "scope " <> Text.unpack ns <> ":" <> Text.unpack kind <> ":" <> Text.unpack ref + NamespaceWide (Namespace ns) -> + "every scope in " <> Text.unpack ns + runRecall :: RecallOptions -> IO () runRecall opts = do connStr <- requireEnv "PG_CONNECTION_STRING" config <- resolveEmbeddingConfig + context <- cliMemoryContext + request <- + case mkRecallQuery opts.target opts.query opts.strategy opts.limit of + Left err -> ioError (userError ("kioku recall: " <> Text.unpack err)) + Right request -> pure request + -- On stderr, not stdout: a script piping hits must see exactly the lines it saw before, while + -- an operator at a terminal should never have to infer from a result count whether they + -- searched one scope or a whole namespace. + hPutStrLn + stderr + ( "kioku recall: searching " + <> describeTarget opts.target + <> ", in memory space " + <> Text.unpack (memorySpaceIdText (memoryContextSpace context)) + ) withNoopAppEnv (defaultConnectionSettings (Text.pack connStr)) \env -> do let model = toEmbeddingModel config - request = - RecallRequest - { scope = opts.scope, - query = opts.query, - strategy = opts.strategy, - maxResults = opts.limit - } result <- runAppIO env do capability <- detectVectorCapability config.dimensions - recall model capability request + recall model capability context request case result of Left storeErr -> ioError (userError ("kioku recall store error: " <> show storeErr)) - Right [] -> putStrLn "(no matches)" - Right hits -> mapM_ (printHit opts.showScores) (zip [(1 :: Int) ..] hits) + Right (Left recallErr) -> ioError (userError ("kioku recall error: " <> show recallErr)) + Right (Right []) -> putStrLn "(no matches)" + Right (Right hits) -> mapM_ (printHit opts.showScores) (zip [(1 :: Int) ..] hits) parseStrategy :: String -> Either String RecallStrategy parseStrategy = \case diff --git a/src/Kioku/Cli/Commands/Scenes.hs b/src/Kioku/Cli/Commands/Scenes.hs --- a/src/Kioku/Cli/Commands/Scenes.hs +++ b/src/Kioku/Cli/Commands/Scenes.hs @@ -8,6 +8,7 @@ import Data.Text qualified as Text import Kioku.Api.Scope (MemoryScope) import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliMemorySpace) import Kioku.Cli.Scope (parseScope) import Kioku.Distill.L2 (SceneRow (..), getScenesByScope) import Kiroku.Store.Connection (defaultConnectionSettings) @@ -32,8 +33,9 @@ runScenes :: ScenesOptions -> IO () runScenes opts = do connStr <- requireEnv "PG_CONNECTION_STRING" + space <- cliMemorySpace withNoopAppEnv (defaultConnectionSettings (Text.pack connStr)) \env -> do - result <- runAppIO env (getScenesByScope opts.scope) + result <- runAppIO env (getScenesByScope space opts.scope) case result of Left storeErr -> ioError (userError ("kioku scenes store error: " <> show storeErr)) Right [] -> putStrLn "(no scenes)" diff --git a/src/Kioku/Cli/Commands/Worker.hs b/src/Kioku/Cli/Commands/Worker.hs --- a/src/Kioku/Cli/Commands/Worker.hs +++ b/src/Kioku/Cli/Commands/Worker.hs @@ -10,13 +10,20 @@ import Control.Exception (SomeException, displayException, try) import Data.Text qualified as Text import Data.Time (getCurrentTime) -import Effectful (IOE, (:>)) -import Kioku.App (AppEnv, runAppIO, withNoopAppEnv) +import Effectful (Eff, IOE, (:>)) +import Kioku.Api.Access (MemorySpaceId, memorySpaceIdText, mkMemorySpaceId) +import Kioku.App (AppEffects, AppEnv, runAppIO, withNoopAppEnv) +import Kioku.Cli.Context (cliContextProvider) import Kioku.Distill.L1 (FindMergeCandidates, recallCandidates) import Kioku.Distill.Runtime (newDistillRuntime) import Kioku.Distill.Timer.Worker (drainKiokuTimers, runKiokuTimerWorkerOnce) import Kioku.Memory.Embedding (EmbeddingConfig (..), resolveEmbeddingConfig, toEmbeddingModel) -import Kioku.Memory.Embedding.Worker (backfillMissingEmbeddings, runEmbeddingWorkerHost) +import Kioku.Memory.Embedding.Worker + ( EmbeddingBackfillScope (..), + backfillMissingEmbeddings, + mkEmbeddingWorkerEnv, + runEmbeddingWorkerHost, + ) import Kioku.Recall.Capability (VectorCapability (..), detectVectorCapability) import Kiroku.Store.Connection (KirokuStore, defaultConnectionSettings, withStore) import Kiroku.Store.Effect (Store) @@ -30,19 +37,27 @@ -- ordered: @--backfill --timers-once@ checked @timersOnce@ first and ignored @--backfill@ -- without a word. As a sum parsed from mutually exclusive alternatives, passing both is a -- parse error. +-- +-- @--space@ belongs to the backfill and to nothing else, so it is a field of that constructor +-- rather than a top-level option. Its default is 'BackfillEverySpace': a worker serves every +-- space in its database, and defaulting to one — @KIOKU_MEMORY_SPACE@, say — would let an +-- operator run a backfill, see a count, and never learn that the other spaces are still +-- unsearchable. data WorkerOptions = WorkerContinuous - | WorkerBackfill + | WorkerBackfill !EmbeddingBackfillScope | WorkerTimersOnce deriving stock (Eq, Show) workerOptionsParser :: Parser WorkerOptions workerOptionsParser = - flag' - WorkerBackfill - ( long "backfill" - <> help "Run one embedding backfill pass and exit (conflicts with --timers-once)" - ) + ( flag' + WorkerBackfill + ( long "backfill" + <> help "Run one embedding backfill pass and exit (conflicts with --timers-once)" + ) + <*> backfillScopeParser + ) <|> flag' WorkerTimersOnce ( long "timers-once" @@ -50,6 +65,24 @@ ) <|> pure WorkerContinuous +backfillScopeParser :: Parser EmbeddingBackfillScope +backfillScopeParser = + maybe BackfillEverySpace BackfillOneSpace + <$> optional + ( option + (eitherReader parseMemorySpace) + ( long "space" + <> metavar "MEMORY_SPACE_ID" + <> help "Backfill only this memory space (default: every space in the database)" + ) + ) + +parseMemorySpace :: String -> Either String MemorySpaceId +parseMemorySpace raw = + case mkMemorySpaceId (Text.pack raw) of + Left err -> Left (Text.unpack err) + Right space -> Right space + runWorker :: WorkerOptions -> IO () runWorker opts = do connStr <- requireEnv "PG_CONNECTION_STRING" @@ -59,8 +92,8 @@ withNoopAppEnv settings \env -> case opts of WorkerTimersOnce -> runTimerOnce env config - WorkerBackfill -> withCapability env config \capability -> - runBackfill env capability config + WorkerBackfill scope -> withCapability env config \capability -> + runBackfill env capability config scope WorkerContinuous -> withCapability env config \capability -> runContinuousWorker env st capability config @@ -87,8 +120,8 @@ mergeCandidateLimit :: Int mergeCandidateLimit = 8 -runBackfill :: AppEnv -> VectorCapability -> EmbeddingConfig -> IO () -runBackfill env capability config = do +runBackfill :: AppEnv -> VectorCapability -> EmbeddingConfig -> EmbeddingBackfillScope -> IO () +runBackfill env capability config scope = do -- Refuse before any event is touched: a backfill under a mismatched dimension count would -- embed every memory in the store and fail the ::vector cast on every single one. case capability of @@ -96,11 +129,17 @@ dieWorker (dimensionMismatchMessage configured actual) _ -> pure () let model = toEmbeddingModel config - result <- runAppIO env (backfillMissingEmbeddings capability model config.dimensions) + result <- runAppIO env (backfillMissingEmbeddings capability (mkEmbeddingWorkerEnv model config.dimensions) scope) case result of Left storeErr -> ioError (userError ("kioku worker backfill store error: " <> show storeErr)) - Right count -> putStrLn ("Backfilled " <> show count <> " memory embeddings.") + Right count -> + putStrLn ("Backfilled " <> show count <> " memory embeddings " <> backfillScopeLabel scope <> ".") +backfillScopeLabel :: EmbeddingBackfillScope -> String +backfillScopeLabel = \case + BackfillEverySpace -> "across every memory space" + BackfillOneSpace space -> "in memory space " <> Text.unpack (memorySpaceIdText space) + -- | Run both pipelines under supervision. -- -- The timer loop used to run on a bare 'forkIO', which produced two silent @@ -116,6 +155,7 @@ runContinuousWorker :: AppEnv -> KirokuStore -> VectorCapability -> EmbeddingConfig -> IO () runContinuousWorker env store capability config = do let model = toEmbeddingModel config + contexts <- cliContextProvider @(Eff AppEffects) case capability of VectorAvailable -> do startupBackfill env capability config @@ -123,7 +163,7 @@ try @SomeException $ race (runTimerLoop env capability config) - (runAppIO env (runEmbeddingWorkerHost store capability model config.dimensions)) + (runAppIO env (runEmbeddingWorkerHost store contexts capability model config.dimensions)) case outcome of -- A halted processor can tear its own machinery down hard enough to -- surface as an exception rather than a clean return (shibuya's halt path @@ -157,7 +197,7 @@ dimensionMismatchMessage configured actual = "embedding dimension mismatch: KIOKU_EMBEDDING_DIMENSIONS=" <> show configured - <> " but kiroku.kioku_memories.embedding is vector(" + <> " but kioku.memories.embedding is vector(" <> show actual <> "); fix the env var or migrate the column" @@ -165,9 +205,20 @@ -- Idempotent, so it is safe on every start. A failure here is only a warning: -- if the database is down, the loops' own retry and exit behavior is the honest -- place for that to surface, not a special case at startup. +-- +-- It covers every space, not @KIOKU_MEMORY_SPACE@: this process is about to subscribe to every +-- space's memory events, so recovering only one space's would leave the others' recall degraded +-- with nothing to say so. startupBackfill :: AppEnv -> VectorCapability -> EmbeddingConfig -> IO () startupBackfill env capability config = do - result <- runAppIO env (backfillMissingEmbeddings capability (toEmbeddingModel config) config.dimensions) + result <- + runAppIO + env + ( backfillMissingEmbeddings + capability + (mkEmbeddingWorkerEnv (toEmbeddingModel config) config.dimensions) + BackfillEverySpace + ) case result of Left storeErr -> hPutStrLn stderr ("kioku worker: startup backfill failed: " <> show storeErr) @@ -182,10 +233,11 @@ runTimerOnce :: AppEnv -> EmbeddingConfig -> IO () runTimerOnce env config = do rt <- newDistillRuntime + contexts <- cliContextProvider @(Eff AppEffects) now <- getCurrentTime result <- runAppIO env do capability <- detectVectorCapability config.dimensions - runKiokuTimerWorkerOnce Nothing rt (mergeCandidateFinder config capability) now + runKiokuTimerWorkerOnce Nothing contexts rt (mergeCandidateFinder config capability) now case result of Left storeErr -> ioError (userError ("kioku timer worker store error: " <> show storeErr)) Right Nothing -> putStrLn "No due kioku distillation timers." @@ -206,9 +258,10 @@ runTimerLoop :: AppEnv -> VectorCapability -> EmbeddingConfig -> IO () runTimerLoop env capability config = do rt <- newDistillRuntime + contexts <- cliContextProvider @(Eff AppEffects) putStrLn "kioku timer worker started." let go failures = do - result <- runAppIO env (drainKiokuTimers Nothing rt (mergeCandidateFinder config capability)) + result <- runAppIO env (drainKiokuTimers Nothing contexts rt (mergeCandidateFinder config capability)) case result of Left storeErr -> do hPutStrLn stderr ("kioku timer worker: store error (will retry): " <> show storeErr) diff --git a/src/Kioku/Cli/Context.hs b/src/Kioku/Cli/Context.hs new file mode 100644 --- /dev/null +++ b/src/Kioku/Cli/Context.hs @@ -0,0 +1,73 @@ +-- | The memory space and principal the CLI acts as. +-- +-- Kioku's core will not write anything without a 'MemoryAccessContext' — a record saying that +-- somebody already decided this caller may do this here. The CLI is a trusted in-process host +-- with no authentication boundary of its own, so it builds one through the deliberately +-- conspicuous 'assumeAuthorizedMemoryContext' rather than by consulting anything. +-- +-- Two environment variables decide what it claims: +-- +-- * @KIOKU_MEMORY_SPACE@ — which memory space the command reads and writes. It defaults to +-- 'legacyMemorySpaceId' (@kioku_legacy@), which is where every row written before memory +-- spaces existed lives, so an unchanged CLI keeps operating on exactly the data it did before. +-- * @KIOKU_ACTOR@ — the principal writes are attributed to. It defaults to @kioku_cli@: the CLI +-- is genuinely the thing acting, and naming it plainly is better than borrowing an identity +-- from a directory the CLI does not talk to. +-- +-- Both are validated, and a malformed value is a startup error rather than a silent fallback — +-- a typo in a memory space name must not quietly send writes somewhere else. +module Kioku.Cli.Context + ( cliMemoryContext, + cliMemoryActor, + cliMemorySpace, + cliContextProvider, + ) +where + +import Data.Text (Text) +import Data.Text qualified as Text +import Kioku.Api.Access + ( MemoryAccessContext, + MemoryActor (..), + MemoryContextProvider, + MemorySpaceId, + assumeAuthorizedContextProvider, + assumeAuthorizedMemoryContext, + legacyMemorySpaceId, + mkMemorySpaceId, + mkPrincipalRef, + ) +import System.Environment (lookupEnv) + +-- | The context every CLI write runs under. +cliMemoryContext :: IO MemoryAccessContext +cliMemoryContext = assumeAuthorizedMemoryContext <$> cliMemorySpace <*> cliMemoryActor + +-- | The provider a CLI-hosted background worker uses. +-- +-- Unlike 'cliMemoryContext' this is not pinned to one space: a worker claims timers for whatever +-- space they were scheduled in, and refusing to serve them would strand the work. What the CLI +-- is asserting by using this is that a process holding its database credentials may act in any +-- space in that database — which is already true of anything with the connection string. +cliContextProvider :: (Applicative m) => IO (MemoryContextProvider m) +cliContextProvider = assumeAuthorizedContextProvider <$> cliMemoryActor + +cliMemorySpace :: IO MemorySpaceId +cliMemorySpace = + resolveEnv "KIOKU_MEMORY_SPACE" legacyMemorySpaceId mkMemorySpaceId + +cliMemoryActor :: IO MemoryActor +cliMemoryActor = + MemoryActor <$> resolveEnv "KIOKU_ACTOR" defaultActor mkPrincipalRef + where + defaultActor = either (error . Text.unpack) id (mkPrincipalRef "kioku_cli") + +resolveEnv :: String -> a -> (Text -> Either Text a) -> IO a +resolveEnv name fallback parse = do + raw <- lookupEnv name + case raw of + Nothing -> pure fallback + Just value -> + case parse (Text.pack value) of + Right parsed -> pure parsed + Left err -> ioError (userError (name <> ": " <> Text.unpack err)) diff --git a/src/Kioku/Cli/Scope.hs b/src/Kioku/Cli/Scope.hs --- a/src/Kioku/Cli/Scope.hs +++ b/src/Kioku/Cli/Scope.hs @@ -1,11 +1,13 @@ module Kioku.Cli.Scope ( parseScope, scopeGrammarError, + parseNamespaceOnly, + namespaceGrammarError, ) where import Data.Text qualified as Text -import Kioku.Api.Scope (MemoryScope (..), mkNamespace, mkScopeKind) +import Kioku.Api.Scope (MemoryScope (..), Namespace, mkNamespace, mkScopeKind) -- | @NAMESPACE@ or @NAMESPACE:KIND:REF@. -- @@ -41,3 +43,20 @@ scopeGrammarError :: String scopeGrammarError = "expected NAMESPACE or NAMESPACE:KIND:REF (REF may contain ':'; NAMESPACE and KIND may not)" + +-- | A bare @NAMESPACE@, with no scope attached. +-- +-- Recall's @--global-bucket@ and @--namespace-wide@ take a namespace rather than a scope, because +-- the scope part is what the flag itself is saying. A colon is therefore rejected with its own +-- message rather than left to 'mkNamespace': someone typing @--namespace-wide mori:repo:web@ is +-- reaching for @--scope@, and saying so is more use than reporting a reserved character. +parseNamespaceOnly :: String -> Either String Namespace +parseNamespaceOnly raw + | Text.isInfixOf ":" text = Left namespaceGrammarError + | otherwise = either (Left . Text.unpack) Right (mkNamespace text) + where + text = Text.pack raw + +namespaceGrammarError :: String +namespaceGrammarError = + "expected a bare NAMESPACE with no scope attached; use --scope NAMESPACE:KIND:REF for one entity" diff --git a/test/Kioku/Cli/ParserSpec.hs b/test/Kioku/Cli/ParserSpec.hs --- a/test/Kioku/Cli/ParserSpec.hs +++ b/test/Kioku/Cli/ParserSpec.hs @@ -6,15 +6,18 @@ import Data.List (isInfixOf) import Data.Text qualified as Text +import Kioku.Api.Access (mkMemorySpaceId) import Kioku.Api.Scope (MemoryScope (..), Namespace (..), ScopeKind (..)) import Kioku.Cli.Commands.Demo (DemoOptions (..), demoOptionsParser, demoScope) import Kioku.Cli.Commands.DemoSession (DemoSessionOptions (..), demoSessionOptionsParser) import Kioku.Cli.Commands.Distill (DistillOptions (..), distillOptionsParser) -import Kioku.Cli.Commands.Recall (RecallOptions (..), recallOptionsParser) +import Kioku.Cli.Commands.Recall (RecallOptions (..), describeTarget, recallOptionsParser) import Kioku.Cli.Commands.Worker (WorkerOptions (..), workerOptionsParser) import Kioku.Cli.Options (redactConnectionString) import Kioku.Cli.Scope (parseScope) import Kioku.Id (genMemoryId, genSessionId, idText) +import Kioku.Memory.Embedding.Worker (EmbeddingBackfillScope (..)) +import Kioku.Recall (RecallTarget (..)) import Options.Applicative import Test.Tasty (TestTree, testGroup) import Test.Tasty.HUnit (assertBool, testCase, (@?=)) @@ -25,6 +28,7 @@ "Kioku.Cli parsers" [ sessionIdTests, scopeTests, + recallTargetTests, limitTests, demoGuardTests, redactionTests, @@ -102,6 +106,77 @@ Left _ -> pure () Right scope -> assertBool (label <> " should not parse, got: " <> show scope) False +-- | Recall is the only command that can be asked to widen what it searches, so each of the three +-- things it can search has exactly one spelling and none of them is a bare namespace. +-- +-- @--scope mori@ used to mean /the whole mori namespace/, which is the opposite of what +-- @kioku scenes --scope mori@ means by the same text. It is now a parse error rather than a +-- silently narrower search: an operator's script would otherwise keep exiting zero while +-- returning a fraction of its rows. +recallTargetTests :: TestTree +recallTargetTests = + testGroup + "recall spells each target exactly once" + [ testCase "--scope takes an entity scope" do + target ["--scope", "mori:repo:web"] + @?= Right (ExactScope (ScopeEntity (Namespace "mori") (ScopeKind "repo") "web")), + testCase "--global-bucket takes a namespace and means the rows with no entity scope" do + target ["--global-bucket", "mori"] @?= Right (ExactScope (ScopeGlobal (Namespace "mori"))), + testCase "--namespace-wide takes a namespace and means every scope under it" do + target ["--namespace-wide", "mori"] @?= Right (NamespaceWide (Namespace "mori")), + -- The distinction the whole initiative exists for: two flags naming the same namespace + -- must not produce the same target. + testCase "the global bucket and the namespace are different targets" do + assertBool + "--global-bucket and --namespace-wide must not agree" + (target ["--global-bucket", "mori"] /= target ["--namespace-wide", "mori"]), + testCase "--scope keeps the shared colon rules" do + target ["--scope", "ops:host:db.internal:5432"] + @?= Right (ExactScope (ScopeEntity (Namespace "ops") (ScopeKind "host") "db.internal:5432")), + testCase "a bare namespace is refused, naming both replacements" do + case target ["--scope", "mori"] of + Right parsed -> assertBool ("--scope mori should not parse, got: " <> show parsed) False + Left err -> do + assertBool ("error should offer --global-bucket: " <> err) ("--global-bucket mori" `isInfixOf` err) + assertBool ("error should offer --namespace-wide: " <> err) ("--namespace-wide mori" `isInfixOf` err) + assertBool + ("error should say which one preserves the old behavior: " <> err) + ("returned before" `isInfixOf` err), + testCase "no target at all lists the three forms" do + case parseWith recallOptionsParser ["query"] of + Right parsed -> assertBool ("a targetless recall should not parse, got: " <> show parsed) False + Left err -> do + assertBool ("failure should say what is missing: " <> err) ("Missing:" `isInfixOf` err) + mapM_ + (\flag -> assertBool ("failure should list " <> flag <> ": " <> err) (flag `isInfixOf` err)) + ["--scope", "--global-bucket", "--namespace-wide"], + testCase "two targets is a parse error naming the second" do + assertConflict "--namespace-wide" (target ["--scope", "a:b:c", "--namespace-wide", "mori"]), + testCase "two targets in the other order is also a parse error" do + assertConflict "--global-bucket" (target ["--namespace-wide", "mori", "--global-bucket", "mori"]), + -- A scope handed to a namespace flag is someone reaching for --scope, and saying so beats + -- reporting a reserved character. + testCase "a scope passed to --namespace-wide points at --scope" do + case target ["--namespace-wide", "mori:repo:web"] of + Right parsed -> assertBool ("should not parse, got: " <> show parsed) False + Left err -> assertBool ("error should point at --scope: " <> err) ("--scope" `isInfixOf` err), + -- The stderr banner is the only place a run says which of the three it did. + testCase "each target describes itself distinctly" do + describeTarget (ExactScope (ScopeGlobal (Namespace "mori"))) @?= "the global bucket of mori" + describeTarget (NamespaceWide (Namespace "mori")) @?= "every scope in mori" + describeTarget (ExactScope (ScopeEntity (Namespace "mori") (ScopeKind "repo") "web")) + @?= "scope mori:repo:web" + ] + where + target extra = fmap (.target) (parseWith recallOptionsParser (["query"] <> extra)) + + assertConflict rejected = \case + Right parsed -> assertBool ("expected a conflict error, got: " <> show parsed) False + Left err -> + assertBool + ("failure should name the conflicting flag " <> rejected <> ": " <> err) + (rejected `isInfixOf` err) + -- | Out-of-range limits are a parse error, not a Postgres error (@--limit -1@ used to reach -- SQL and come back as @LIMIT must not be negative@). limitTests :: TestTree @@ -131,7 +206,7 @@ ] where recallWith extra = - parseWith recallOptionsParser (["query", "--scope", "mori"] <> extra) + parseWith recallOptionsParser (["query", "--namespace-wide", "mori"] <> extra) distillWith sid extra = parseWith distillOptionsParser (["session", Text.unpack (idText sid)] <> extra) @@ -196,8 +271,16 @@ "worker one-shot modes are mutually exclusive" [ testCase "no flags means the continuous worker" do parseWith workerOptionsParser [] @?= Right WorkerContinuous, - testCase "--backfill" do - parseWith workerOptionsParser ["--backfill"] @?= Right WorkerBackfill, + testCase "--backfill covers every space unless one is named" do + parseWith workerOptionsParser ["--backfill"] @?= Right (WorkerBackfill BackfillEverySpace), + testCase "--backfill --space bounds the pass to one space" do + parseWith workerOptionsParser ["--backfill", "--space", "space_prod"] + @?= Right (WorkerBackfill (BackfillOneSpace (spaceNamed "space_prod"))), + -- A space id that no caller could have constructed must not become one here either: the + -- backfill would then scan for a partition value the database cannot hold and report a + -- confident zero. + testCase "--space rejects a malformed memory space id" do + assertParseFailure (parseWith workerOptionsParser ["--backfill", "--space", "bad:space"]), testCase "--timers-once" do parseWith workerOptionsParser ["--timers-once"] @?= Right WorkerTimersOnce, testCase "both flags is a parse error" do @@ -212,3 +295,10 @@ assertBool ("failure should name the conflicting flag " <> rejected <> ": " <> err) (rejected `isInfixOf` err) + + assertParseFailure = \case + Right mode -> assertBool ("expected a parse error, got: " <> show mode) False + Left _ -> pure () + + spaceNamed raw = + either (error . Text.unpack) id (mkMemorySpaceId (Text.pack raw)) diff --git a/test/Kioku/Cli/RecallEndToEndSpec.hs b/test/Kioku/Cli/RecallEndToEndSpec.hs new file mode 100644 --- /dev/null +++ b/test/Kioku/Cli/RecallEndToEndSpec.hs @@ -0,0 +1,202 @@ +-- | @kioku recall@ against a real database, driven the way an operator drives it. +-- +-- "Kioku.Cli.ParserSpec" proves each flag produces the intended 'RecallTarget', and +-- @Kioku.RecallTargetSpec@ in @kioku-core@ proves what each target returns. What neither can see +-- is the seam between them: that the parsed target is the one handed to the query, that +-- @KIOKU_MEMORY_SPACE@ bounds the search, and that the run announces itself on stderr without +-- putting a word on stdout. +-- +-- The command runs as a subprocess rather than through 'Kioku.Cli.Commands.Recall.runRecall' in +-- process. @stdout@, @stderr@ and the environment are process-wide, and tasty runs cases +-- concurrently, so redirecting them here would race every other case in the suite. A subprocess +-- gets its own three. +-- +-- Every case is @--strategy keyword@, which plans no vector channel and therefore never embeds: +-- the suite needs a database, not an embedding endpoint. +module Kioku.Cli.RecallEndToEndSpec (tests) where + +import Control.Monad.IO.Class (liftIO) +import Data.List (isInfixOf) +import Data.Text (Text) +import Data.Text qualified as Text +import Data.Time (UTCTime, getCurrentTime) +import Effectful (Eff, IOE, (:>)) +import Effectful.Error.Static (Error) +import Kioku.Api.Access + ( MemoryAccessContext, + MemoryActor (..), + MemorySpaceId, + assumeAuthorizedMemoryContext, + memoryContextRecordedActor, + memorySpaceIdText, + mkMemorySpaceId, + mkPrincipalRef, + ) +import Kioku.Api.Scope (MemoryScope (..), Namespace (..), ScopeKind (..)) +import Kioku.Api.Types (Confidence (..), MemoryType (..)) +import Kioku.App (runAppIO, withNoopAppEnv) +import Kioku.Id (genMemoryId) +import Kioku.Memory qualified as Memory +import Kioku.Memory.Domain (RecordMemoryData (..)) +import Kioku.Migrations.TestSupport (withKiokuMigratedDatabase) +import Kiroku.Store.Connection (defaultConnectionSettings) +import Kiroku.Store.Effect (Store) +import Kiroku.Store.Effect.Resource (KirokuStoreResource) +import Kiroku.Store.Error (StoreError) +import System.Environment (getEnvironment) +import System.Exit (ExitCode (..)) +import System.Process (CreateProcess (..), proc, readCreateProcessWithExitCode) +import Test.Tasty (TestTree, testGroup) +import Test.Tasty.HUnit (assertBool, assertFailure, testCase, (@?=)) + +tests :: TestTree +tests = + testGroup + "kioku recall end to end" + [ testCase "each flag reaches the database as its own target, inside one space" targetsReachPostgres + ] + +-- | Two spaces holding the same namespace and the same two scopes, with content that names which +-- space and which scope it came from — so a row appearing in the wrong answer is unmistakable. +alphaSpace :: MemorySpaceId +alphaSpace = spaceNamed "space_cli_alpha" + +betaSpace :: MemorySpaceId +betaSpace = spaceNamed "space_cli_beta" + +namespaceText :: Text +namespaceText = "mori" + +entityScope :: MemoryScope +entityScope = ScopeEntity (Namespace namespaceText) (ScopeKind "repo") "web" + +globalScope :: MemoryScope +globalScope = ScopeGlobal (Namespace namespaceText) + +targetsReachPostgres :: IO () +targetsReachPostgres = withKiokuMigratedDatabase \connStr -> do + now <- getCurrentTime + seeded <- + withNoopAppEnv (defaultConnectionSettings connStr) \env -> + runAppIO env do + seed alphaSpace globalScope "alpha global checklist" now + seed alphaSpace entityScope "alpha web checklist" now + seed betaSpace globalScope "beta global checklist" now + seed betaSpace entityScope "beta web checklist" now + case seeded of + Left storeErr -> assertFailure ("seeding failed: " <> show storeErr) + Right () -> pure () + + let recallIn space args = runKioku connStr space (["recall", "checklist", "--strategy", "keyword"] <> args) + + -- The global bucket is the target that had no representation at all before this initiative. + (globalCode, globalOut, _) <- recallIn alphaSpace ["--global-bucket", Text.unpack namespaceText] + globalCode @?= ExitSuccess + assertContains "the global bucket" "alpha global" globalOut + assertMissing "the global bucket" "alpha web" globalOut + + (entityCode, entityOut, _) <- recallIn alphaSpace ["--scope", Text.unpack namespaceText <> ":repo:web"] + entityCode @?= ExitSuccess + assertContains "the entity scope" "alpha web" entityOut + assertMissing "the entity scope" "alpha global" entityOut + + (wideCode, wideOut, wideErr) <- recallIn alphaSpace ["--namespace-wide", Text.unpack namespaceText] + wideCode @?= ExitSuccess + assertContains "namespace-wide" "alpha global" wideOut + assertContains "namespace-wide" "alpha web" wideOut + + -- No target may cross the partition, however wide it is. The other space's rows differ only in + -- one word, and the widest target is the one that would reach them if anything could. + mapM_ (\out -> assertMissing "any target in the alpha space" "beta" out) [globalOut, entityOut, wideOut] + + -- The same widest target under the other space's context answers with that space's rows. + (betaCode, betaOut, betaErr) <- recallIn betaSpace ["--namespace-wide", Text.unpack namespaceText] + betaCode @?= ExitSuccess + assertContains "namespace-wide in the beta space" "beta global" betaOut + assertContains "namespace-wide in the beta space" "beta web" betaOut + assertMissing "namespace-wide in the beta space" "alpha" betaOut + + -- The banner says what was searched, and says it where a pipe will not pick it up. + assertContains "the stderr banner" "every scope in mori" wideErr + assertContains "the stderr banner" (Text.unpack (memorySpaceIdText alphaSpace)) wideErr + assertContains "the stderr banner" (Text.unpack (memorySpaceIdText betaSpace)) betaErr + assertMissing "stdout" "kioku recall: searching" wideOut + + -- The one spelling whose meaning would have changed fails before it reaches the database. + (bareCode, bareOut, bareErr) <- recallIn alphaSpace ["--scope", Text.unpack namespaceText] + assertBool ("a bare --scope namespace must fail, got: " <> show bareCode) (bareCode /= ExitSuccess) + assertContains "the ambiguity error" "--global-bucket mori" bareErr + assertContains "the ambiguity error" "--namespace-wide mori" bareErr + assertMissing "the ambiguity error's stdout" "checklist" bareOut + +seed :: + (IOE :> es, KirokuStoreResource :> es, Store :> es, Error StoreError :> es) => + MemorySpaceId -> + MemoryScope -> + Text -> + UTCTime -> + Eff es () +seed space scope content now = do + memoryId <- liftIO genMemoryId + let context = contextFor space + recorded <- + Memory.recordWithContext + context + RecordMemoryData + { memorySpaceId = space, + actorPrincipal = memoryContextRecordedActor context, + ownerPrincipal = Nothing, + memoryId, + agentId = "cli-test", + sessionId = Nothing, + scope, + memoryType = MemoryPreference, + content, + priority = 50, + confidence = HighConfidence, + tags = mempty, + supersedes = Nothing, + recordedAt = now + } + case recorded of + Left writeErr -> liftIO (assertFailure ("recordWithContext: " <> show writeErr)) + Right _ -> pure () + +-- | Run the built @kioku@ binary. Cabal puts it on @PATH@ through @build-tool-depends@. +-- +-- The connection string and memory space are replaced rather than added, so a developer's +-- exported @PG_CONNECTION_STRING@ cannot redirect the test at their own database. +runKioku :: Text -> MemorySpaceId -> [String] -> IO (ExitCode, String, String) +runKioku connStr space args = do + inherited <- getEnvironment + let overridden = + [ (name, value) + | (name, value) <- inherited, + name `notElem` ["PG_CONNECTION_STRING", "KIOKU_MEMORY_SPACE"] + ] + <> [ ("PG_CONNECTION_STRING", Text.unpack connStr), + ("KIOKU_MEMORY_SPACE", Text.unpack (memorySpaceIdText space)) + ] + readCreateProcessWithExitCode (proc "kioku" args) {env = Just overridden} "" + +assertContains :: String -> String -> String -> IO () +assertContains label needle haystack = + assertBool + (label <> " should mention " <> show needle <> ", got:\n" <> haystack) + (needle `isInfixOf` haystack) + +assertMissing :: String -> String -> String -> IO () +assertMissing label needle haystack = + assertBool + (label <> " must not mention " <> show needle <> ", got:\n" <> haystack) + (not (needle `isInfixOf` haystack)) + +contextFor :: MemorySpaceId -> MemoryAccessContext +contextFor space = assumeAuthorizedMemoryContext space testActor + +testActor :: MemoryActor +testActor = + MemoryActor (either (error . Text.unpack) id (mkPrincipalRef "agent_01h9xk3v7hf8b9c0d1e2f3g4h5")) + +spaceNamed :: Text -> MemorySpaceId +spaceNamed raw = either (error . Text.unpack) id (mkMemorySpaceId raw) diff --git a/test/Main.hs b/test/Main.hs --- a/test/Main.hs +++ b/test/Main.hs @@ -1,7 +1,14 @@ module Main where import Kioku.Cli.ParserSpec qualified as ParserSpec -import Test.Tasty (defaultMain) +import Kioku.Cli.RecallEndToEndSpec qualified as RecallEndToEndSpec +import Test.Tasty (defaultMain, testGroup) main :: IO () -main = defaultMain ParserSpec.tests +main = + defaultMain $ + testGroup + "kioku-cli" + [ ParserSpec.tests, + RecallEndToEndSpec.tests + ]