-
Notifications
You must be signed in to change notification settings - Fork 372
fix(q): persist cache state to end 100% cache miss streak #28473
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
Changes from all commits
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 | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -129,6 +129,45 @@ This workflow was triggered from a comment on discussion #${{ github.event.discu | |||||||||||||||||||||
| - Is this a general optimization request? | ||||||||||||||||||||||
| 3. **Identify Target Workflows**: Determine which workflows to analyze (specific ones or all) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #### Cache Initialization | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| The cache root for this workflow is `/tmp/gh-aw/cache-memory` (restored by the `cache-memory: true` setup step). | ||||||||||||||||||||||
| Q's state lives in a workflow-specific subdirectory: `/tmp/gh-aw/cache-memory/q/`. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Load prior state from cache at startup so this run can build on previous findings: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| # Cache root: /tmp/gh-aw/cache-memory (restored by the cache-memory setup step) | ||||||||||||||||||||||
| # Q-specific state lives in the /q/ subdirectory inside that root | ||||||||||||||||||||||
| CACHE_DIR=/tmp/gh-aw/cache-memory/q | ||||||||||||||||||||||
| mkdir -p "$CACHE_DIR" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if [ -f "$CACHE_DIR/state.json" ]; then | ||||||||||||||||||||||
| echo "Cache restored — loading prior Q state" | ||||||||||||||||||||||
| cat "$CACHE_DIR/state.json" | jq '{workflows_analyzed: (.workflows_analyzed | length), patterns_count: (.patterns | length), last_run: .last_run}' | ||||||||||||||||||||||
|
||||||||||||||||||||||
| cat "$CACHE_DIR/state.json" | jq '{workflows_analyzed: (.workflows_analyzed | length), patterns_count: (.patterns | length), last_run: .last_run}' | |
| cat "$CACHE_DIR/state.json" | jq '{workflows_analyzed_count: (.workflows_analyzed | length), patterns_count: (.patterns | length), last_run: .last_run}' |
Copilot
AI
Apr 25, 2026
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.
patterns: ($prev_pat + $new_pat | unique_by(.name)) does not actually merge pattern content across runs: if a pattern with the same name appears again (e.g., with additional workflows), unique_by keeps only one copy and can drop the new data. If you want patterns to accumulate, consider grouping by .name and merging fields (at least unioning the workflows arrays, and deciding whether to prefer the newest description).
| patterns: ($prev_pat + $new_pat | unique_by(.name)), | |
| patterns: ( | |
| ($prev_pat + $new_pat) | |
| | group_by(.name) | |
| | map({ | |
| name: .[0].name, | |
| description: (map(.description // empty) | last // ""), | |
| workflows: (map(.workflows // []) | add | unique) | |
| }) | |
| ), |
Copilot
AI
Apr 25, 2026
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.
The list of variables to populate includes last_run, but the snippet sets NOW=... and writes last_run: $now inside state.json. As written, there is no last_run shell variable for the agent to populate—this bullet should refer to NOW (the shell variable) or to the last_run JSON field, otherwise it’s misleading.
| - `last_run`: Filesystem-safe UTC timestamp (`YYYY-MM-DD-HH-MM-SS`) — no colons | |
| - `NOW`: Filesystem-safe UTC timestamp (`YYYY-MM-DD-HH-MM-SS`) — no colons; this is written to the `last_run` field in `state.json` |
Copilot
AI
Apr 25, 2026
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.
The text says the merge "accumulates history across runs without growing unbounded", but processed_run_ids (and potentially workflows_analyzed) will still grow indefinitely as new unique values appear over time. If bounding size is a goal, add an explicit retention policy (e.g., keep only the most recent N run IDs / patterns, or prune by age using last_run).
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.
@copilot validate the cache path matches the actual cache folder location
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.
Validated in commit
85cfe1d. The compiled lock file confirms the cache save/restore operates onpath: /tmp/gh-aw/cache-memory(the root). The prompt now uses/tmp/gh-aw/cache-memory/q/— a workflow-specific subdirectory nested inside that root, which follows the same convention asdaily-compiler-quality(/tmp/gh-aw/cache-memory/compiler-quality/). Both the Phase 0 and Phase 5.5 snippets now include inline comments making this relationship explicit.