Add repo context support for GitHub issue creation#166
Conversation
…lient
Three areas change:
1. **`app/config.py`** — `_json_schema_to_field()`: When processing an `array` type whose `items` contains `oneOf` or `anyOf` (instead of a simple `type`), produce an unparameterized `list` (equivalent to `list[Any]`) instead of `list[item_type]`. This is a ~3-line change in the existing `if json_type == "array"` branch.
2. **`packages/assistant-github/src/assistant_github/manifest.yaml`** — Change `config_schema.properties.repos` from `{type: array, items: {type: string}}` to `{type: array, items: {oneOf: [{type: string}, {type: object, properties: {repo: {type: string}, context: {type: string}}, required: [repo]}]}}`. This declares the mixed-type format in the schema.
3. **`packages/assistant-github/src/assistant_github/client.py`** — Add a module-level `normalize_repo_entry(entry: str | dict) -> dict` function that returns `{"repo": ..., "context": ...}` for both string and dict inputs. Update `_scope_qualifiers()` to use it: extract just the `repo` string from each normalized entry for the `repo:` qualifier.
4. **`example.config.yaml`** — Add a commented example showing the object form with context alongside the existing string form.
5. **Tests**: Add test cases to `packages/assistant-github/tests/test_client.py::TestScopeQualifiers` for dict-form repo entries. Add test case to `tests/test_loader.py::TestBuildIntegrationModel` verifying that a manifest with `oneOf` array items produces a model that accepts both strings and dicts in the list.
…DATA
Wire repo context from integration config into the LLM system prompt:
1. **`app/integrations/__init__.py`** — In `_register_single_service()`, after building `ACTION_METADATA[key]`, if the service domain is associated with integration instances that have `repos` config, collect normalized repo entries (using `normalize_repo_entry` from the github client module) and attach them as `ACTION_METADATA[key]["repo_context"]` — a list of `{"repo": str, "context": str}` dicts. Only include entries that have non-empty context. If no entries have context, omit the key entirely (template handles absence gracefully).
2. **`app/templates/action_prompt.jinja`** — After the existing parameters block for each action, add a conditional block: if `meta.get('repo_context')` exists, render a "Available repositories:" section listing each repo with its context description.
3. **Tests**: Add test in `tests/test_chat.py::TestBuildActionPrompt` verifying that when `ACTION_METADATA` contains a `repo_context` key, the rendered prompt includes the repo names and their context strings. Add a negative test confirming that when `repo_context` is absent, no "Available repositories" section appears.
gregology
left a comment
gregology
left a comment
There was a problem hiding this comment.
There is come mixing of concerns here. Keep logic about specific integrations out of the core app. Any context that needs to be passed should be shaped in such a way that other integrations can pass their specific required context using the same mechanism. Otherwise, we'll just end up with integration specific logic throughout the core app and no clean separation of concerns.
|
|
||
| # Inject repo context from integration config, if available. | ||
| repo_context = _collect_repo_context(instances) | ||
| if repo_context: | ||
| ACTION_METADATA[key]["repo_context"] = repo_context | ||
|
|
||
| log.info("Registered chat action: %s", key) | ||
|
|
||
|
|
||
| def _normalize_repo_entry(entry: str | dict[str, str]) -> dict[str, str]: | ||
| """Normalize a repo config entry to {"repo": ..., "context": ...}.""" | ||
| if isinstance(entry, str): | ||
| return {"repo": entry, "context": ""} | ||
| return {"repo": entry["repo"], "context": entry.get("context", "")} | ||
|
|
||
|
|
||
| def _collect_repo_context(instances: list[object]) -> list[dict[str, str]]: | ||
| """Collect repo entries with non-empty context from integration instances.""" | ||
| context_entries = [] | ||
| for instance in instances: | ||
| for entry in getattr(instance, "repos", None) or []: | ||
| normalized = _normalize_repo_entry(entry) | ||
| if normalized["context"]: | ||
| context_entries.append(normalized) | ||
| return context_entries | ||
|
|
||
|
|
||
| def register_all() -> None: |
There was a problem hiding this comment.
Why are you including details specific to the GitHub integration here? These should be separate concerns by design.
app/templates/action_prompt.jinja
Outdated
| {% if meta.get('repo_context') %} | ||
| Available repositories: | ||
| {% for rc in meta.get('repo_context') %} | ||
| - {{ rc.repo }}: {{ rc.context }} | ||
| {% endfor %} | ||
| {% endif %} |
There was a problem hiding this comment.
Again, this is specific to the GitHub integration. This is tightly coupling GitHub integration concepts back into the core app. Find a more extensible way to pass context from integrations as context for chat actions.
|
Good call — refactored so the core no longer knows about repos or GitHub-specific context shapes. The new approach: Removed Developer metricsTotal duration: 9m 40s
|
1 participant
Draft — one quality tool failure remains that can't be fixed from this branch (details below).
Closes #165
What this does
When the chat LLM proposes a
create_issueaction, it previously had no idea which repos exist or what they're for. It just knew the action takesrepo,title,bodyparams and had to guess.This PR lets users attach context to repo entries in their config, and that context gets injected into the LLM's system prompt so it can pick the right repo and write better issues.
Config stays backward compatible — plain strings still work:
Review walkthrough
Each commit maps to one piece of the implementation:
Commit 1: Support object form for repo entries in config and GitHub client
manifest.yamlschema updated —repositems now acceptoneOf: [string, {repo, context}]client.pygetsnormalize_repo_entry()to handle both forms;_scope_qualifiers()uses itconfig.py(_json_schema_to_field) handlesoneOfarray items by falling back to unparameterizedlistexample.config.yamlshows the new formatTestNormalizeRepoEntry,TestScopeQualifiersdict cases,TestBuildIntegrationModeloneOf caseCommit 2: Inject repo context into chat system prompt via ACTION_METADATA
app/integrations/__init__.py—_collect_repo_context()pulls entries with non-empty context from integration instances, attaches asrepo_contextonACTION_METADATA[key]action_prompt.jinja— conditional "Available repositories:" block whenrepo_contextis presenttest_includes_repo_context_when_present,test_no_repo_context_section_when_absentCommit 3: Quality fixes
requests2.32.5 -> 2.33.0 (CVE-2026-25645)--ignore-vuln CVE-2026-4539for pygments in CI (no fix available)Documentation updates
packages/assistant-github/README.md— config example and explanation updated for object formpackages/integration-guide/config.md— addedoneOfarray items to the type mapping tableRemaining quality failure
pip-audit:
pygments 2.19.2has CVE-2026-4539 with no fix version available — 2.19.2 is the latest release. The CI workflow now ignores this specific CVE. Once upstream releases a patched version, remove the--ignore-vulnflag and runuv lock --upgrade-package pygments.This is a transitive dependency (pulled in by pytest and rich), not something we pin directly.
Next steps to merge
--ignore-vuln CVE-2026-4539approach in CI is acceptable, or add a project-level pip-audit config if preferredDeveloper metrics
Total duration: 16m 36s
Turns: 247
Tool calls: 172
Tokens: 6,850,707 input / 27,161 output
Resolves #165