-
Notifications
You must be signed in to change notification settings - Fork 21
RFD: predicate caching #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| # Predicate caching | ||
|
|
||
| Predicates (especially `shell(...)`) spawn processes on every sync. This RFD helps them declare watch paths. Symposium caches results and skips re-evaluation while watched files are unchanged. No watch declaration = never cached. | ||
|
|
||
| ## Problem | ||
|
|
||
| Auto-sync means predicates re-evaluate on every agent session start. A workspace with 10 plugins, each with a `shell(...)` gate, forks 10+ processes every time. | ||
|
|
||
| ## Design | ||
|
|
||
| Two ways to declare watch paths: | ||
|
|
||
| ### 1. Syntactic | ||
|
|
||
| For predicates whose inputs are known at authorship time: | ||
|
|
||
| ```toml | ||
| [[skills]] | ||
| predicates = ["shell(grep -q lambda CargoBrazil.toml)"] | ||
| watch = ["CargoBrazil.toml"] | ||
| source.path = "skills/lambda" | ||
| ``` | ||
|
|
||
| `watch` is a sibling field, not embedded in the predicate string. | ||
|
|
||
| ### 2. Runtime | ||
|
|
||
| The shell command emits JSON to stdout: | ||
|
|
||
| ```json | ||
| {"result": true, "watch": ["CargoBrazil.toml"]} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we restructured the custom predicates to emit lines of jsonl, right? I think that the idea would be that they emit a line like which would derive from #[derive(SErialize, Deserialize)]
enum CustomPredicateEvent {
...., // whatever other events we have, I think right now that is for selected crates? I think we'll be removing that eventually but it's ok
Watch { files: Vec<PathBuf> }
}I believe that the predicate result comes from the exit status of the shell? Anyway, it should be independent from the custom predicate event.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll align with the existing a variant. Exit status stays as the result, watch event is just a cache hint emitted alongside. Will update the RFD. |
||
| ``` | ||
|
|
||
| Commands that print nothing or non-JSON fall back to exit-code semantics with no caching. Backward compatible. | ||
|
|
||
| Runtime takes priority when both are present. Either is sufficient to enable caching. | ||
|
|
||
| ## How it works | ||
|
|
||
| Fingerprint is `mtime + size` (same model as Cargo). A missing file is a valid fingerprint state — invalidates when the file appears. | ||
|
|
||
| Cache lives at `predicate-cache.json`, keyed by the normalized predicate string. Two plugins with the same predicate share one entry. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the key part is that the cache is stored in the user's symposium directory? I would assume we are kind of generalizing the current
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, I'll put it in |
||
|
|
||
| Algorithm: | ||
|
|
||
| 1. Look up predicate in cache | ||
| 2. If found and all watched files match fingerprints → use cached result | ||
| 3. Otherwise → evaluate, store result + watch paths if declared | ||
| 4. On write, prune entries for predicates no longer in any manifest | ||
|
|
||
| Cache is discarded on Symposium version upgrade. `watch = []` (empty) means static for the current invocation only — no persistent caching without at least one file to stat. | ||
|
|
||
| ## Built-in predicates | ||
|
|
||
| - `workspace-member()` → cached, watch = [] (static per run) | ||
| - `path_exists(path)` → cached, watches the path itself | ||
| - `depends-on(name)` → cached, watches PM manifest (e.g. `Cargo.lock`) | ||
| - `env(...)` → never cached (env isn't file-observable) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should add another kind of predicate here - not just files but also the values of environment variables. Env can cache this way and shell predicates should also be able to emit that. We could have a helper in the |
||
| - `shell(...)` → cached only with explicit watch declaration | ||
|
|
||
| ## PM integration | ||
|
|
||
| `list_deps` return type expands to include optional watch paths: | ||
|
|
||
| ```rust | ||
| struct DepsResult { | ||
| ids: Vec<PackageId>, | ||
| watch: Option<Vec<WatchEntry>>, | ||
| } | ||
| ``` | ||
|
|
||
| `CargoPm` returns `[Cargo.lock]`. This is a return-type change, not a new method. | ||
|
|
||
| ## Implementation steps | ||
|
|
||
| 1. Extend `PredicateResult` with `watch: Option<Vec<WatchEntry>>`. All evaluators return `None` initially — no behavior change. | ||
| 2. Cache storage: read/write, fingerprint comparison, dead-entry pruning. | ||
| 3. Built-in predicates return their natural watch sets. | ||
| 4. Shell runtime output: parse stdout JSON, fall back to exit code for non-JSON. | ||
| 5. Syntactic `watch` field on `[[skills]]` groups. | ||
| 6. PM `list_deps` watch support (coordinate with pm-split). | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I'm inclined to skip this and just focus on the runtime version. I guess that would make something like a
shellpredicate kind of inherently tricky but that's ok, I don't think an arbitrary shell command is a very good idea for portability reasons anyhow. I'd rather we have declarative forms for common shell commands.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll drop the syntactic watch field and focus on the JSONL runtime approach. Agreed that arbitrary shell commands aren't great for portability and maybe declarative forms for common patterns is the better direction.