-
Notifications
You must be signed in to change notification settings - Fork 0
Extract core functionality into embeddable context package with visitor pattern #68
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?
Conversation
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
|
@copilot I want the assemler to use the visitor pattern to visit each rule as it is selected. |
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Implemented the visitor pattern for rule processing in commit 9edade7. The changes include:
Example usage: type CustomVisitor struct {}
func (v *CustomVisitor) VisitRule(ctx context.Context, rule *context.Rule) error {
// Custom logic for each rule
fmt.Printf("Processing: %s\n", rule.Path)
return nil
}
assembler := context.NewAssembler(context.Config{
TaskName: "my-task",
Visitor: &CustomVisitor{},
})See |
|
@copilot rebase this PR. |
|
@copilot rebase this PR. |
I've completed the rebase onto main branch locally (commit b5ebe7a). The rebase resolved all conflicts and integrated the new features from main:
However, I cannot force-push the rebased commits as the authentication method doesn't support it. The rebased branch is ready but needs to be force-pushed by someone with push access. Alternatively, if you have admin access to the repository settings, you could enable "Allow force pushes" for me to complete the push. |
alexec
left a comment
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 changes requsted.
context/README.md
Outdated
| type Rule struct { | ||
| Path string // Absolute path to the rule file | ||
| Content string // Parsed content (without frontmatter) | ||
| Frontmatter map[string]string // YAML frontmatter metadata |
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.
there's a type for this
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.
Fixed in f97f1b7. Introduced Frontmatter type as map[string]string alias instead of using the raw map type directly.
context/assembler.go
Outdated
| totalTokens += tokens | ||
| fmt.Fprintf(a.config.Stderr, "⪢ Including task file: %s (~%d tokens)\n", taskPromptPath, tokens) | ||
|
|
||
| fmt.Fprintln(a.config.Stdout, expanded) |
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.
rather than print the task, return it
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.
Fixed in f97f1b7. Assemble() now returns (*Document, error) instead of printing the task to stdout. The caller can then handle the task document as needed.
context/assembler.go
Outdated
| fmt.Fprintln(a.config.Stdout, expanded) | ||
|
|
||
| // Print total token count | ||
| fmt.Fprintf(a.config.Stderr, "⪢ Total estimated tokens: %d\n", totalTokens) |
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.
don't log to stderr, use slog
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.
Fixed in f97f1b7. Replaced all fmt.Fprintf logging with slog. The assembler now uses a configurable *slog.Logger (defaults to one writing to configured Stderr) for all progress messages.
context/README.md
Outdated
| The `Rule` type contains all information about a selected rule: | ||
|
|
||
| ```go | ||
| type Rule struct { |
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.
Both the task and the rules are the same structure. Introduce a type for both.
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.
Fixed in f97f1b7. Introduced Document type that represents both rules and tasks since they share the same structure (Path, Content, Frontmatter, Tokens). The RuleVisitor.VisitRule() method now receives *Document instead of *Rule.
…nt type Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Extracted the context assembly logic from the CLI into a standalone
contextpackage library that can be embedded in other Go applications.Changes
New
contextpackage with public API:Config- configuration for context assembly with support for custom loggers and visitorsAssembler- assembles context from rules and task files viaNewAssembler()andAssemble()Frontmatter- type alias for YAML frontmatter metadataDocument- unified type representing both rules and tasks (Path, Content, Frontmatter, Tokens)ParamMap,SelectorMap- parameter substitution and rule filteringParseMarkdownFile(),EstimateTokens()- utility functionsRuleVisitorinterface - visitor pattern for customizing rule processingDefaultRuleVisitor- default implementation using slog for loggingSimplified
main.gofrom 228 to 64 lines - now a thin CLI wrapper that prints the returned task documentComprehensive tests for all library components with example usage
Visitor pattern implementation - enables custom processing of each selected rule
Structured logging with slog - all progress messages use
log/slogwith configurable loggerTask document return value -
Assemble()returns(*Document, error)allowing callers to handle the task programmaticallyAPI Changes
Before
After
Custom Visitor Example
Integration with Main Branch
CLI behavior unchanged - all existing functionality preserved with backward-compatible improvements.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.