-
Notifications
You must be signed in to change notification settings - Fork 29
feat: Add work queue and context generation systems (Phase 3.1 - PR 1.3) #29
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
Conversation
Adds work queue management and context generation infrastructure: Work Queue System (state/): - queue-manual.md: Template for manually maintained work queue - queue-generated.md: Template for auto-generated queue - README.md: Complete documentation of two-queue system Scripts Documentation (scripts/): - README.md: Comprehensive documentation of context generation, task management, and autonomous run scripts - Note tasks.py as optional (provided by gptme-contrib) Main README updates: - Document state/ and scripts/ directories - Explain work queue and context generation systems - Link to detailed documentation Fixes: - Remove broken tasks.py symlink (will be added in PR 2.3) - Update documentation to note tasks.py as optional Part of gptme-agent-template Phase 3.1 implementation. See ErikBjare/bob#167 for context.
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.
Important
Looks good to me! 👍
Reviewed everything up to 75615a2 in 1 minute and 29 seconds. Click for details.
- Reviewed
442lines of code in7files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. ARCHITECTURE.md:21
- Draft comment:
Clarify that the optional task management CLI is provided by gptme-contrib and consider linking to its docs. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. scripts/README.md:12
- Draft comment:
The directory tree lists an optional 'tasks.py', but other docs reference 'generate-work-queue.py'. Consider unifying the naming/location for clarity. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. state/README.md:48
- Draft comment:
The generation command references './scripts/tasks/generate-work-queue.py', which isn’t shown in the scripts tree. Clarify its source/installation (likely via gptme-contrib). - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_XGG95Opz50iNVN4i
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Greptile OverviewGreptile SummaryAdds comprehensive work queue management and context generation infrastructure to gptme-agent-template, enabling better autonomous operation support. Key Changes:
Integration Points:
All changes are well-documented with clear separation between mandatory and optional components. Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Template as gptme-agent-template
participant Fork as fork.sh
participant Context as scripts/context.sh
participant Queue as state/queue-*.md
participant Agent as Autonomous Agent
Note over User,Template: PR Changes Overview
User->>Template: Add work queue system
Template->>Template: Create state/queue-manual.md
Template->>Template: Create state/queue-generated.md
Template->>Template: Add state/README.md
User->>Template: Add scripts documentation
Template->>Template: Create scripts/README.md
Template->>Template: Update scripts/context.sh
Note right of Template: Handle optional tasks.py
User->>Template: Make tasks.py optional
Template->>Template: Remove broken symlink
Template->>Template: Update .pre-commit-config.yaml
Template->>Template: Update ARCHITECTURE.md
Template->>Template: Update fork.sh
Note over Fork,Agent: Usage Flow
User->>Fork: Run fork.sh to create agent
Fork->>Fork: Initialize git repo
Fork->>Fork: Add gptme-contrib submodule
Fork->>Fork: Check if tasks.py available
alt tasks.py exists
Fork->>Fork: Set timestamp on task
else tasks.py not available
Fork->>Fork: Skip timestamp (optional)
end
Agent->>Context: Request context generation
Context->>Context: Generate header
Context->>Context: Check if tasks.py exists
alt tasks.py exists
Context->>Context: Show task status
else tasks.py not available
Context->>Context: Show installation note
end
Context->>Queue: Read queue-manual.md
Queue-->>Context: Strategic priorities
Context-->>Agent: Full context bundle
Agent->>Queue: Update during autonomous run
Queue->>Queue: Update "Current Run"
Queue->>Queue: Update "Planned Next"
|
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.
Additional Comments (3)
-
scripts/context.sh, line 32 (link)logic:
tasks.pyno longer exists (symlink removed in this PR), will cause script to fail -
.pre-commit-config.yaml, line 51 (link)logic:
tasks.pyremoved in this PR, pre-commit hook will fail -
fork.sh, line 130-134 (link)logic: Submodule added but not initialized before using
tasks.py- script will fail
7 files reviewed, 3 comments
- scripts/context.sh: Add conditional check for tasks.py existence - .pre-commit-config.yaml: Remove tasks.py validation hook (optional tool) - fork.sh: Initialize gptme-contrib submodule before use, make tasks.py call optional tasks.py is documented as optional (from gptme-contrib) but was referenced unconditionally in three locations, breaking functionality when not present. Fixes critical issues identified in PR review (greptile 1/5 confidence score). Co-authored-by: Bob <[email protected]>
Fixed Critical Issues Identified by GreptileAddressed the 3 critical issues that were causing 1/5 confidence score: 1. scripts/context.sh - Made tasks.py optional
2. .pre-commit-config.yaml - Removed tasks.py validation hook
3. fork.sh - Initialize submodule and make tasks.py optional
Result
The PR design correctly treats tasks.py as optional (from gptme-contrib), but the implementation had unconditional references that would break core workflows. This commit fixes those references while preserving the intended optional behavior. Commit: 653e9f1 |
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.
Caution
Changes requested ❌
Reviewed 653e9f1 in 2 minutes and 0 seconds. Click for details.
- Reviewed
66lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .pre-commit-config.yaml:46
- Draft comment:
Removed tasks.py validation hook looks intentional. Ensure the README or docs clarify that tasks.py is now optional and how to run its check if needed. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. fork.sh:136
- Draft comment:
Good update: wrapping tasks.py execution in a file-existence check. Consider using -x to ensure it’s executable, to avoid potential permission issues. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment suggests a potential improvement to check for executable permissions. However, this seems speculative - the comment says "to avoid potential permission issues" but doesn't demonstrate that there's an actual problem. The code is checking if the file exists before trying to execute it, and if it's not executable, the script would fail with a clear error message anyway. The change being made is to add graceful handling when tasks.py doesn't exist (making it optional), not to handle permission issues. Using-xwould be more strict and might cause false negatives if the file exists but isn't executable yet (though it should be). This feels like a speculative suggestion rather than addressing a clear issue with the change. The comment might be valid if tasks.py commonly has permission issues in practice. The-xcheck would be more precise since the script is trying to execute the file. However, I don't have evidence that this is actually a problem, and the comment is speculative ("potential permission issues"). While-xmight be technically more precise, the comment is speculative and doesn't point to a concrete issue with the change. The rules state "Do NOT make speculative comments, such as 'If X, then Y is an issue', Only comment if it is definitely an issue." This comment uses the word "potential" which indicates speculation. If there were permission issues, they would be caught when the script tries to execute tasks.py anyway. This comment should be deleted because it's speculative ("potential permission issues") rather than pointing to a definite issue. The change is about making tasks.py optional when it doesn't exist, not about handling permission issues. There's no evidence that permission issues are a real problem here.
3. fork.sh:129
- Draft comment:
Initializing the gptme-contrib submodule recursively is a positive improvement for nested submodules. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that a change is a positive improvement without offering any specific guidance or questions.
Workflow ID: wflow_LnP02IjuxsMH3pNb
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
scripts/context.sh
Outdated
| echo -e "Output of \`$SCRIPT_DIR/tasks.py status --compact\` command:\n" | ||
| $SCRIPT_DIR/tasks.py status --compact | ||
| # tasks.py is optional (provided by gptme-contrib submodule) | ||
| if [ -f "$SCRIPT_DIR/tasks.py" ]; then |
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.
For robustness, consider checking execution permission (e.g. using -x) instead of just file existence for tasks.py.
| if [ -f "$SCRIPT_DIR/tasks.py" ]; then | |
| if [ -x "$SCRIPT_DIR/tasks.py" ]; then |
Fixes broken links in forked agents by: - Add state/ directory to fork.sh copy list (core infrastructure) - Add lessons/TEMPLATE.md to fork.sh copy list - Update lessons/README.md with TEMPLATE.md reference from origin/master - Fix template file copy to use explicit paths instead of glob Root cause: PR merges into origin/master which includes lessons-system changes, but fork.sh didn't copy all referenced files and directories. Local verification: make check-names passes Test fork verification: pre-commit checks pass including markdown links Addresses CI failure in PR #29
| # redundant? already validated by validate-task-frontmatter? | ||
| # should combine them | ||
| - id: validate-tasks | ||
| name: Validate task metadata | ||
| description: Validates YAML frontmatter in task files | ||
| entry: ./scripts/tasks.py check | ||
| language: system | ||
| types: [markdown] | ||
| pass_filenames: false | ||
| #files: ^tasks/.*\.md$ |
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.
This should be included, or silently pass/skip if tasks.py doesn't exist/broken symlink.
|
@greptileai review |
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.
10 files reviewed, 1 comment
README.md
Outdated
|
|
||
| **[`scripts/`](./scripts/)**: Automation and utilities | ||
| - `context.sh` - Main context generation orchestrator | ||
| - `tasks.py` - Task management CLI |
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.
style: Clarify that tasks.py is optional here - the current wording suggests it's always available
| - `tasks.py` - Task management CLI | |
| - `tasks.py` - Task management CLI (optional, from gptme-contrib) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 88:88
Comment:
**style:** Clarify that `tasks.py` is optional here - the current wording suggests it's always available
```suggestion
- `tasks.py` - Task management CLI (optional, from gptme-contrib)
```
How can I resolve this? If you propose a fix, please make it concise.- scripts/context.sh: use -x check for executable instead of -f - .pre-commit-config.yaml: add validation hook that gracefully handles missing tasks.py - README.md: clarify tasks.py is optional from gptme-contrib Addresses review comments from Greptile bot: - scripts/context.sh line 32: robustness check - .pre-commit-config.yaml line 55: include validation or skip if missing - README.md line 88: clarify optional status Co-authored-by: Bob <[email protected]>
|
✅ Addressed review feedback on optional tasks.py handling Changes made:
The validation hook now:
Ready for re-review! 🚀 |
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.
Important
Looks good to me! 👍
Reviewed beacef4 in 31 seconds. Click for details.
- Reviewed
43lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .pre-commit-config.yaml:45
- Draft comment:
Using the '-x' check in the hook ensures that tasks.py is not only present but also executable. Confirm that tasks.py will always have execute permissions in user environments. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. README.md:88
- Draft comment:
The README now documents that tasks.py is optional and comes from gptme-contrib. This helps clarify its usage for users. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. scripts/context.sh:32
- Draft comment:
Changing the check from '-f' to '-x' correctly ensures that tasks.py is executable before running it. Ensure that installation instructions clear that tasks.py must have executable permissions. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_n46QSW4wXGfFBOUI
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Summary
Adds work queue management and context generation infrastructure to gptme-agent-template.
Work Queue System (state/)
Manual Queue (queue-manual.md):
Generated Queue (queue-generated.md):
Documentation (state/README.md):
Scripts Documentation (scripts/)
scripts/README.md:
Main README Updates
Fixes
Part of Phase 3.1
This is PR 1.3 in the gptme-agent-template sync:
See ErikBjare/bob#167 for complete context.
Important
Introduces work queue and context generation systems, updates documentation, and fixes minor issues in the gptme-agent-template.
queue-manual.mdfor manually maintained work queue with strategic context.queue-generated.mdfor auto-generated queue from tasks and GitHub.state/README.mdwith a guide to the two-queue system and CASCADE workflow integration.scripts/README.mddocumenting context generation, task management, and autonomous run infrastructure.context.shto handle optionaltasks.pyfor task status.state/andscripts/directories.tasks.pysymlink.tasks.pyas optional.ARCHITECTURE.md.This description was created by
for beacef4. You can customize this summary. It will automatically update as commits are pushed.