feat: task-aware generator profiles (workload-selectable, tier-aware) - #177
Conversation
…op apply_recipe auto-seed
…ect dashboard-settability + 8GB docs; trim stale apply_recipe docstring - memory_extractor.py L259-263: switch from get_memory_model to resolve_memory_model so extraction routes through the generator-profile resolver (pin > profile > tier) rather than only reading the raw config pin - README.md + spec: remove false claim that generator_profile can be set from the dashboard Settings panel; consumer-scope controls are read-only in the dashboard and set via CLI only - README.md: fix balanced profile 8 GB description (qwen3.5:9b on 12/8 GB, llama3.1:8b on 4 GB only) - recipes.py apply_recipe docstring: drop stale clause about writing generator model to config (auto-seed removed in 728be1e) - tests/test_memory_extractor_model_resolution.py: regression coverage for the resolve_memory_model code path
…test Replace vacuous isolation test with a spy-based test that calls process_conversation_turn directly. The spy monkeypatches extract_facts_with_llm to capture the model kwarg and raise a sentinel, short-circuiting before kg access. Test 1 asserts the model is NOT "default" (balanced@gpu-12gb -> qwen3.5:9b), which fails on the pre-fix get_memory_model() path. Test 2 verifies the "default" fallback when the tier is absent from the balanced map.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds a task-aware generator profile system ( ChangesGenerator Profile System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…e-branch review, regression caught+fixed), PR #177 open for sign-off
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| print(f"error: unknown profile {profile_id!r}", file=sys.stderr) | ||
| return 1 | ||
| if agent: | ||
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) |
There was a problem hiding this comment.
CRITICAL: _generator_profile_set does not catch AgentNotFoundError raised by agents.set_agent_generator_profile(agent, ...). Running taosmd generator-profile set <id> --agent does-not-exist will print a Python traceback to stderr instead of a clean error message and a non-zero exit. Other CLI sites that touch per-agent state (cli.py:69, 108, 636) all wrap this exception. Mirror that pattern here.
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) | |
| if agent: | |
| try: | |
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) | |
| except agents.AgentNotFoundError as exc: | |
| print(f"error: {exc}", file=sys.stderr) | |
| return 1 | |
| print(f"agent {agent}: generator profile = {profile_id}") |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| resolved_model = extraction_model | ||
| if not resolved_model or resolved_model == "default": | ||
| resolved_model = get_memory_model() or "default" | ||
| resolved_model = resolve_memory_model() or "default" |
There was a problem hiding this comment.
CRITICAL: resolve_memory_model() is called here without an agent argument, so generator_profiles.resolve_generator never sees the per-agent profile and silently falls through to the global profile. The new per-agent branch (and its test_per_agent_beats_global coverage) is therefore unreachable from the production extraction path. Either pass agent_name through process_conversation_turn → resolve_memory_model(agent=...), or document the limitation explicitly so users do not assume --agent works for fact extraction. This is the exact regression shape that commit f203a77 (the memory_extractor fix) was meant to guard against — but it only fixed the global profile path.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return "balanced" | ||
|
|
||
|
|
||
| def resolve_generator(agent: str | None = None, *, fallback: str | None = None, |
There was a problem hiding this comment.
WARNING: Doc/code divergence on the resolution precedence. The PR description and config.resolve_memory_model (config.py:256-261) advertise pin > profile > recipe generator > fallback > retrieval-only, but this function never consults recipe.generator["model"]. The previous behaviour (auto-seeded by apply_recipe) is also gone (recipes.py:393-398 removed the seed). The result: a user who applies a recipe that names a generator no longer gets that generator — they get whatever the active profile says for the detected tier, with no recipe visibility. Either re-implement the recipe-generator branch in resolve_generator (e.g. recipes.get_recipe(_agents.get_agent(agent)["applied_recipe_id"]).generator.get("model", "")) or update the spec docstring + README + design doc to drop the "recipe generator" rung from the precedence list.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| pid = rec.get("generator_profile_id") | ||
| return pid if isinstance(pid, str) and pid.strip() else None | ||
|
|
||
| def set_agent_generator_profile(self, name: str, profile_id: str | None) -> dict: |
There was a problem hiding this comment.
WARNING: set_agent_generator_profile (and the matching config.set_generator_profile at config.py:131-144) persist any non-empty string without validating it against generator_profiles.get_profile(...). A typo like "factual-recal", "Balanced", or "balanced " (trailing space — .strip() does save you here) is silently written to disk; the CLI confirms "set successfully" and the profile is then ignored at resolution time, falling back to default_profile_id(). The CLI wrapper does call gp.get_profile(profile_id) first, so the bug is only reachable from direct Python callers (tests, library users, the agent-facing API). Add a registry lookup in both setters and raise ValueError for unknown ids so the failure mode is loud.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "generator_profile": Control( | ||
| id="generator_profile", label="Generator profile", | ||
| category="quality", scope="consumer", type="choice", | ||
| config_key="generator_profile", |
There was a problem hiding this comment.
SUGGESTION: config_key="generator_profile" is a flat (top-level) key, while every other control in this table uses a dotted path (controls.prefer_verified, vector_memory.late_interaction, answer.self_verify, vector_memory.embed_model, controls.fusion, controls.adjacent_turns, controls.reranker). The asymmetry is harmless today (consumer-scope controls are not part of get_runtime_overrides), but it is a trap for any future code that walks Control.config_key to read/write a value, and the test in tests/test_generator_profile_control.py even uses a or "generator_profile" in str(schema) OR-clause that papers over a real schema-shape ambiguity. Either pick a dotted form (e.g. controls.generator_profile) or drop the field's config_key value to a documented placeholder and fix the test.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (19 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 70.6K · Output: 8.3K · Cached: 719.5K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
tests/test_generator_profile_cli.py (1)
4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the per-agent CLI branch too.
_generator_profile_sethas a separate persistence path whenagentis provided, but this file only protects the global path and the unknown-profile error. A small test that registers an agent, calls_generator_profile_set(..., agent="alice"), and assertsagents.get_agent_generator_profile(...)would lock down the new CLI surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_generator_profile_cli.py` around lines 4 - 18, The current CLI tests only cover the global generator profile path in _generator_profile_set and miss the per-agent branch. Add a test that registers an agent, calls cli._generator_profile_set with a valid profile and agent="alice", and then verifies the value through agents.get_agent_generator_profile to lock down the agent-specific persistence behavior alongside the existing unknown-profile check.tests/test_generator_profile_agent.py (1)
9-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the field is actually removed on clear.
The last assertion only checks the accessor contract. This would still pass if
set_agent_generator_profile(..., None)wrote""instead of deletinggenerator_profile_id, so it won't catch a persistence regression in the clear path.Suggested test tightening
agents.set_agent_generator_profile("alice", None, data_dir=tmp_path) assert agents.get_agent_generator_profile("alice", data_dir=tmp_path) is None + rec = agents.AgentRegistry(tmp_path).get_agent("alice") + assert "generator_profile_id" not in rec🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_generator_profile_agent.py` around lines 9 - 12, The clear-path test for agent generator profiles is too weak because it only checks get_agent_generator_profile, so it can miss cases where set_agent_generator_profile(..., None) stores an empty value instead of removing generator_profile_id. Tighten the test in test_generator_profile_agent.py by asserting the persisted record/state for alice after clearing no longer contains generator_profile_id, using the existing set_agent_generator_profile and get_agent_generator_profile flow as the setup. This should verify the field is actually deleted from storage, not just interpreted as None by the accessor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@taosmd/agents.py`:
- Around line 332-343: The public setter set_agent_generator_profile currently
writes any non-empty string into agent records, so validate the provided profile
id before saving it. Reuse the same generator-profile lookup/validation used by
the CLI path (or call the generator-profile resolver/registry check) and reject
unknown ids instead of persisting them. Keep the clear/remove behavior for None
or blank values, and apply the same validation to the other affected assignment
path noted in the diff.
In `@taosmd/cli.py`:
- Around line 1777-1784: The generator-profile dispatch in main() is not passing
args.data_dir into the helper calls, so list/show/set use the default store
instead of the user-selected directory. Update the generator-profile branch to
thread args.data_dir through _generator_profile_list, _generator_profile_show,
and _generator_profile_set, matching their existing data_dir parameter. Keep the
profile_id and agent arguments unchanged while ensuring every generator-profile
subcommand operates on the same data directory from args.data_dir.
- Around line 169-177: The _generator_profile_set CLI path currently lets
agents.set_agent_generator_profile() raise AgentNotFoundError for an unknown
--agent, which produces a traceback instead of a clean CLI error. Update
_generator_profile_set to catch AgentNotFoundError around the agent update call,
print a normal error: message to stderr that includes the agent name, and return
a nonzero exit code while keeping the existing profile validation and success
print behavior intact.
In `@taosmd/config.py`:
- Around line 256-265: resolve_memory_model currently hides per-agent overrides
because it always calls generator_profiles.resolve_generator() without an agent
context. Update resolve_memory_model to accept and forward an agent parameter,
then make taosmd.memory_extractor.process_conversation_turn() pass the active
agent through this shim so the new per-agent precedence in resolve_generator()
can be reached on the extraction path.
- Around line 120-146: The config setter currently accepts any non-empty string,
so bad profile ids can be persisted and later bypass the documented default
behavior. Update set_generator_profile() in taosmd/config.py to reject unknown
ids at the boundary by validating against the registered generator profiles
before writing. Keep the clear=True path unchanged, and ensure
get_generator_profile()/resolve_generator() only ever see known profile ids or
None.
In `@taosmd/memory_extractor.py`:
- Around line 259-263: The memory model resolution in process_conversation_turn
is still using only the global default path, so agent-specific generator-profile
overrides are ignored. Update the resolve_memory_model call path to accept and
use agent_name (or the appropriate agent/profile context) when extraction_model
is unset or "default", and ensure the fallback logic still preserves the global
default only when no per-agent override exists. Keep the fix localized around
process_conversation_turn and resolve_memory_model so fact extraction honors
generator-profile set --agent for that agent.
In `@tests/test_generator_profile_control.py`:
- Around line 21-24: The test in test_generator_profile_in_schema is too weak
because it string-matches the schema instead of verifying the actual controls
payload. Update the assertion to inspect the structured result from
controls.controls_schema() directly and confirm generator_profile appears as a
real control entry in the returned controls list, using the existing
controls_schema symbol and control item ids rather than str(schema).
- Around line 15-18: The test for controls.validate_control is too broad because
it catches any Exception instead of the specific contract. Update
test_generator_profile_rejects_unknown to assert ValueError from
validate_control when passed the "generator_profile" control with an invalid
value like "nope", using the same function and test name to keep the expectation
precise.
In `@tests/test_memory_extractor_model_resolution.py`:
- Around line 99-103: The assertion message in the memory model resolution test
contains unnecessary f-string prefixes, which triggers lint error F541. Update
the assertion in tests/test_memory_extractor_model_resolution.py so the
multi-line message used by the assert on model != "default" is plain string text
rather than f-strings; keep the wording intact and remove the stray f prefixes
from that assertion message.
---
Nitpick comments:
In `@tests/test_generator_profile_agent.py`:
- Around line 9-12: The clear-path test for agent generator profiles is too weak
because it only checks get_agent_generator_profile, so it can miss cases where
set_agent_generator_profile(..., None) stores an empty value instead of removing
generator_profile_id. Tighten the test in test_generator_profile_agent.py by
asserting the persisted record/state for alice after clearing no longer contains
generator_profile_id, using the existing set_agent_generator_profile and
get_agent_generator_profile flow as the setup. This should verify the field is
actually deleted from storage, not just interpreted as None by the accessor.
In `@tests/test_generator_profile_cli.py`:
- Around line 4-18: The current CLI tests only cover the global generator
profile path in _generator_profile_set and miss the per-agent branch. Add a test
that registers an agent, calls cli._generator_profile_set with a valid profile
and agent="alice", and then verifies the value through
agents.get_agent_generator_profile to lock down the agent-specific persistence
behavior alongside the existing unknown-profile check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 599652e3-b121-44d3-bab9-4e18437a2301
📒 Files selected for processing (19)
README.mddocs/benchmarks.mddocs/superpowers/specs/2026-06-24-task-aware-generator-profiles-design.mdtaosmd/agents.pytaosmd/cli.pytaosmd/config.pytaosmd/controls.pytaosmd/generator_profiles.pytaosmd/memory_extractor.pytaosmd/recipes.pytests/test_config_memory_model.pytests/test_generator_profile_agent.pytests/test_generator_profile_cli.pytests/test_generator_profile_config.pytests/test_generator_profile_control.pytests/test_generator_profiles.pytests/test_generator_resolution.pytests/test_memory_extractor_model_resolution.pytests/test_recipes.py
| def set_agent_generator_profile(self, name: str, profile_id: str | None) -> dict: | ||
| """Set or clear the per-agent generator-profile id (None/'' clears).""" | ||
| data = self._read() | ||
| for a in data["agents"]: | ||
| if a["name"] == name: | ||
| if profile_id and profile_id.strip(): | ||
| a["generator_profile_id"] = profile_id.strip() | ||
| else: | ||
| a.pop("generator_profile_id", None) | ||
| self._write(data) | ||
| return dict(a) | ||
| raise AgentNotFoundError(f"agent {name!r} is not registered") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate generator-profile ids before writing agent records.
Unlike the CLI path, this public setter accepts any truthy string. A bad generator_profile_id then causes taosmd.generator_profiles.resolve_generator() to miss both the agent profile and the expected default behavior for that agent, falling through to fallback / retrieval-only.
Suggested fix
def set_agent_generator_profile(self, name: str, profile_id: str | None) -> dict:
"""Set or clear the per-agent generator-profile id (None/'' clears)."""
+ from . import generator_profiles # lazy: avoids agents<->profiles cycle
data = self._read()
for a in data["agents"]:
if a["name"] == name:
- if profile_id and profile_id.strip():
- a["generator_profile_id"] = profile_id.strip()
+ if isinstance(profile_id, str) and profile_id.strip():
+ normalized = profile_id.strip()
+ if generator_profiles.get_profile(normalized) is None:
+ raise ValueError(f"unknown generator profile {normalized!r}")
+ a["generator_profile_id"] = normalized
else:
a.pop("generator_profile_id", None)
self._write(data)
return dict(a)Also applies to: 620-621
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/agents.py` around lines 332 - 343, The public setter
set_agent_generator_profile currently writes any non-empty string into agent
records, so validate the provided profile id before saving it. Reuse the same
generator-profile lookup/validation used by the CLI path (or call the
generator-profile resolver/registry check) and reject unknown ids instead of
persisting them. Keep the clear/remove behavior for None or blank values, and
apply the same validation to the other affected assignment path noted in the
diff.
| def _generator_profile_set(profile_id: str, agent=None, data_dir=None) -> int: | ||
| from . import generator_profiles as gp | ||
| from . import config, agents | ||
| if gp.get_profile(profile_id) is None: | ||
| print(f"error: unknown profile {profile_id!r}", file=sys.stderr) | ||
| return 1 | ||
| if agent: | ||
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) | ||
| print(f"agent {agent}: generator profile = {profile_id}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle unknown --agent without a traceback.
Line 176 calls agents.set_agent_generator_profile(), which raises AgentNotFoundError for an unregistered agent. Right now that escapes the CLI and prints a Python traceback instead of a normal error: message and nonzero exit.
Suggested fix
def _generator_profile_set(profile_id: str, agent=None, data_dir=None) -> int:
from . import generator_profiles as gp
from . import config, agents
if gp.get_profile(profile_id) is None:
print(f"error: unknown profile {profile_id!r}", file=sys.stderr)
return 1
if agent:
- agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir)
+ try:
+ agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir)
+ except agents.AgentNotFoundError as exc:
+ print(f"error: {exc}", file=sys.stderr)
+ return 1
print(f"agent {agent}: generator profile = {profile_id}")
else:
config.set_generator_profile(profile_id, data_dir=data_dir)
print(f"global generator profile = {profile_id}")
return 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _generator_profile_set(profile_id: str, agent=None, data_dir=None) -> int: | |
| from . import generator_profiles as gp | |
| from . import config, agents | |
| if gp.get_profile(profile_id) is None: | |
| print(f"error: unknown profile {profile_id!r}", file=sys.stderr) | |
| return 1 | |
| if agent: | |
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) | |
| print(f"agent {agent}: generator profile = {profile_id}") | |
| def _generator_profile_set(profile_id: str, agent=None, data_dir=None) -> int: | |
| from . import generator_profiles as gp | |
| from . import config, agents | |
| if gp.get_profile(profile_id) is None: | |
| print(f"error: unknown profile {profile_id!r}", file=sys.stderr) | |
| return 1 | |
| if agent: | |
| try: | |
| agents.set_agent_generator_profile(agent, profile_id, data_dir=data_dir) | |
| except agents.AgentNotFoundError as exc: | |
| print(f"error: {exc}", file=sys.stderr) | |
| return 1 | |
| print(f"agent {agent}: generator profile = {profile_id}") | |
| else: | |
| config.set_generator_profile(profile_id, data_dir=data_dir) | |
| print(f"global generator profile = {profile_id}") | |
| return 0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/cli.py` around lines 169 - 177, The _generator_profile_set CLI path
currently lets agents.set_agent_generator_profile() raise AgentNotFoundError for
an unknown --agent, which produces a traceback instead of a clean CLI error.
Update _generator_profile_set to catch AgentNotFoundError around the agent
update call, print a normal error: message to stderr that includes the agent
name, and return a nonzero exit code while keeping the existing profile
validation and success print behavior intact.
| if args.cmd == "generator-profile": | ||
| if args.generator_profile_cmd == "list": | ||
| return _generator_profile_list() | ||
| if args.generator_profile_cmd == "show": | ||
| return _generator_profile_show(args.profile_id) | ||
| if args.generator_profile_cmd == "set": | ||
| return _generator_profile_set(args.profile_id, agent=args.agent) | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Thread --data-dir through the generator-profile dispatch.
These helpers all accept data_dir, but main() calls them without args.data_dir. So taosmd --data-dir /tmp/x generator-profile set factual-recall still reads/writes the default store, and list/show report the wrong active profile.
Suggested fix
if args.cmd == "generator-profile":
if args.generator_profile_cmd == "list":
- return _generator_profile_list()
+ return _generator_profile_list(data_dir=args.data_dir)
if args.generator_profile_cmd == "show":
- return _generator_profile_show(args.profile_id)
+ return _generator_profile_show(args.profile_id, data_dir=args.data_dir)
if args.generator_profile_cmd == "set":
- return _generator_profile_set(args.profile_id, agent=args.agent)
+ return _generator_profile_set(
+ args.profile_id,
+ agent=args.agent,
+ data_dir=args.data_dir,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if args.cmd == "generator-profile": | |
| if args.generator_profile_cmd == "list": | |
| return _generator_profile_list() | |
| if args.generator_profile_cmd == "show": | |
| return _generator_profile_show(args.profile_id) | |
| if args.generator_profile_cmd == "set": | |
| return _generator_profile_set(args.profile_id, agent=args.agent) | |
| if args.cmd == "generator-profile": | |
| if args.generator_profile_cmd == "list": | |
| return _generator_profile_list(data_dir=args.data_dir) | |
| if args.generator_profile_cmd == "show": | |
| return _generator_profile_show(args.profile_id, data_dir=args.data_dir) | |
| if args.generator_profile_cmd == "set": | |
| return _generator_profile_set( | |
| args.profile_id, | |
| agent=args.agent, | |
| data_dir=args.data_dir, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/cli.py` around lines 1777 - 1784, The generator-profile dispatch in
main() is not passing args.data_dir into the helper calls, so list/show/set use
the default store instead of the user-selected directory. Update the
generator-profile branch to thread args.data_dir through
_generator_profile_list, _generator_profile_show, and _generator_profile_set,
matching their existing data_dir parameter. Keep the profile_id and agent
arguments unchanged while ensuring every generator-profile subcommand operates
on the same data directory from args.data_dir.
| def get_generator_profile(data_dir=None) -> str | None: | ||
| """Return the active global generator-profile id, or None if unset.""" | ||
| pid = _read(data_dir).get(_GENERATOR_PROFILE_KEY) | ||
| if isinstance(pid, str) and pid.strip(): | ||
| return pid | ||
| return None | ||
|
|
||
|
|
||
| def set_generator_profile(profile_id: str, clear: bool = False, data_dir=None) -> None: | ||
| """Persist the active global generator-profile id. | ||
|
|
||
| Args: | ||
| profile_id: a registered profile id. Ignored when clear is True. | ||
| clear: when True, remove the setting (unset). | ||
|
|
||
| Raises: | ||
| ValueError: when clear is False and profile_id is not a non-empty str. | ||
| """ | ||
| data = _read(data_dir) | ||
| if clear: | ||
| data.pop(_GENERATOR_PROFILE_KEY, None) | ||
| _write(data, data_dir) | ||
| return | ||
| if not isinstance(profile_id, str) or not profile_id.strip(): | ||
| raise ValueError("profile_id must be a non-empty string") | ||
| data[_GENERATOR_PROFILE_KEY] = profile_id.strip() | ||
| _write(data, data_dir) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject unknown profile ids at the config boundary.
cli._generator_profile_set() already rejects unknown ids, but set_generator_profile() still persists any non-empty string. Once that happens, taosmd.generator_profiles.resolve_generator() treats the bad id as a miss and skips the documented balanced default, so a typo here can silently drop generation to fallback / retrieval-only.
Suggested fix
def get_generator_profile(data_dir=None) -> str | None:
"""Return the active global generator-profile id, or None if unset."""
pid = _read(data_dir).get(_GENERATOR_PROFILE_KEY)
if isinstance(pid, str) and pid.strip():
- return pid
+ from . import generator_profiles # lazy: avoids config<->profiles cycle
+ pid = pid.strip()
+ if generator_profiles.get_profile(pid) is not None:
+ return pid
return None
@@
def set_generator_profile(profile_id: str, clear: bool = False, data_dir=None) -> None:
@@
if not isinstance(profile_id, str) or not profile_id.strip():
raise ValueError("profile_id must be a non-empty string")
- data[_GENERATOR_PROFILE_KEY] = profile_id.strip()
+ from . import generator_profiles # lazy: avoids config<->profiles cycle
+ profile_id = profile_id.strip()
+ if generator_profiles.get_profile(profile_id) is None:
+ raise ValueError(f"unknown generator profile {profile_id!r}")
+ data[_GENERATOR_PROFILE_KEY] = profile_id
_write(data, data_dir)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_generator_profile(data_dir=None) -> str | None: | |
| """Return the active global generator-profile id, or None if unset.""" | |
| pid = _read(data_dir).get(_GENERATOR_PROFILE_KEY) | |
| if isinstance(pid, str) and pid.strip(): | |
| return pid | |
| return None | |
| def set_generator_profile(profile_id: str, clear: bool = False, data_dir=None) -> None: | |
| """Persist the active global generator-profile id. | |
| Args: | |
| profile_id: a registered profile id. Ignored when clear is True. | |
| clear: when True, remove the setting (unset). | |
| Raises: | |
| ValueError: when clear is False and profile_id is not a non-empty str. | |
| """ | |
| data = _read(data_dir) | |
| if clear: | |
| data.pop(_GENERATOR_PROFILE_KEY, None) | |
| _write(data, data_dir) | |
| return | |
| if not isinstance(profile_id, str) or not profile_id.strip(): | |
| raise ValueError("profile_id must be a non-empty string") | |
| data[_GENERATOR_PROFILE_KEY] = profile_id.strip() | |
| _write(data, data_dir) | |
| def get_generator_profile(data_dir=None) -> str | None: | |
| """Return the active global generator-profile id, or None if unset.""" | |
| pid = _read(data_dir).get(_GENERATOR_PROFILE_KEY) | |
| if isinstance(pid, str) and pid.strip(): | |
| from . import generator_profiles # lazy: avoids config<->profiles cycle | |
| pid = pid.strip() | |
| if generator_profiles.get_profile(pid) is not None: | |
| return pid | |
| return None | |
| def set_generator_profile(profile_id: str, clear: bool = False, data_dir=None) -> None: | |
| """Persist the active global generator-profile id. | |
| Args: | |
| profile_id: a registered profile id. Ignored when clear is True. | |
| clear: when True, remove the setting (unset). | |
| Raises: | |
| ValueError: when clear is False and profile_id is not a non-empty str. | |
| """ | |
| data = _read(data_dir) | |
| if clear: | |
| data.pop(_GENERATOR_PROFILE_KEY, None) | |
| _write(data, data_dir) | |
| return | |
| if not isinstance(profile_id, str) or not profile_id.strip(): | |
| raise ValueError("profile_id must be a non-empty string") | |
| from . import generator_profiles # lazy: avoids config<->profiles cycle | |
| profile_id = profile_id.strip() | |
| if generator_profiles.get_profile(profile_id) is None: | |
| raise ValueError(f"unknown generator profile {profile_id!r}") | |
| data[_GENERATOR_PROFILE_KEY] = profile_id | |
| _write(data, data_dir) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/config.py` around lines 120 - 146, The config setter currently accepts
any non-empty string, so bad profile ids can be persisted and later bypass the
documented default behavior. Update set_generator_profile() in taosmd/config.py
to reject unknown ids at the boundary by validating against the registered
generator profiles before writing. Keep the clear=True path unchanged, and
ensure get_generator_profile()/resolve_generator() only ever see known profile
ids or None.
| def resolve_memory_model(fallback: str | None = None, data_dir=None) -> str | None: | ||
| """Return the global memory model if set, else ``fallback``. | ||
| """Resolve the active generator model: pin > profile(tier) > fallback. | ||
|
|
||
| Consumers call this so an unset global transparently falls back to | ||
| their existing default. Standalone installs that never set a model | ||
| keep working exactly as before. | ||
| Delegates to generator_profiles.resolve_generator (lazy import to avoid a | ||
| cycle). Returns None when resolution yields the empty (retrieval-only) | ||
| value AND no fallback was given, preserving the historical None contract. | ||
| """ | ||
| model = get_memory_model(data_dir) | ||
| return model if model is not None else fallback | ||
| from . import generator_profiles # lazy: avoids config<->profiles cycle | ||
| resolved = generator_profiles.resolve_generator(fallback=fallback, data_dir=data_dir) | ||
| return resolved or None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Per-agent overrides are unreachable through this shim.
taosmd.memory_extractor.process_conversation_turn() now resolves through resolve_memory_model(), but this helper has no agent parameter and always calls resolve_generator() globally. That makes the new per-agent precedence in taosmd.generator_profiles.resolve_generator() impossible to reach on the extraction path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/config.py` around lines 256 - 265, resolve_memory_model currently
hides per-agent overrides because it always calls
generator_profiles.resolve_generator() without an agent context. Update
resolve_memory_model to accept and forward an agent parameter, then make
taosmd.memory_extractor.process_conversation_turn() pass the active agent
through this shim so the new per-agent precedence in resolve_generator() can be
reached on the extraction path.
| from .config import resolve_memory_model # noqa: PLC0415 | ||
|
|
||
| resolved_model = extraction_model | ||
| if not resolved_model or resolved_model == "default": | ||
| resolved_model = get_memory_model() or "default" | ||
| resolved_model = resolve_memory_model() or "default" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Per-agent generator-profile overrides are still bypassed here.
process_conversation_turn() has agent_name, but resolve_memory_model() only receives fallback/data_dir in the contract shown here. That means this path can only resolve against the global/default profile, so generator-profile set --agent ... never affects fact extraction for that agent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taosmd/memory_extractor.py` around lines 259 - 263, The memory model
resolution in process_conversation_turn is still using only the global default
path, so agent-specific generator-profile overrides are ignored. Update the
resolve_memory_model call path to accept and use agent_name (or the appropriate
agent/profile context) when extraction_model is unset or "default", and ensure
the fallback logic still preserves the global default only when no per-agent
override exists. Keep the fix localized around process_conversation_turn and
resolve_memory_model so fact extraction honors generator-profile set --agent for
that agent.
| def test_generator_profile_rejects_unknown(): | ||
| import pytest | ||
| with pytest.raises(Exception): | ||
| controls.validate_control("generator_profile", "nope") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test file ==\n'
git ls-files 'tests/test_generator_profile_control.py' 'taosmd/controls.py' | cat
printf '\n== test excerpt ==\n'
sed -n '1,80p' tests/test_generator_profile_control.py
printf '\n== validator excerpt ==\n'
sed -n '1,220p' taosmd/controls.py
printf '\n== locate validate_control references ==\n'
rg -n "def validate_control|validate_control\(" -S .Repository: jaylfc/taosmd
Length of output: 14768
Assert ValueError here. pytest.raises(Exception) is too broad; this validator raises ValueError for invalid choices, so the test should pin that contract.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 17-17: Do not assert blind exception: Exception
(B017)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_generator_profile_control.py` around lines 15 - 18, The test for
controls.validate_control is too broad because it catches any Exception instead
of the specific contract. Update test_generator_profile_rejects_unknown to
assert ValueError from validate_control when passed the "generator_profile"
control with an invalid value like "nope", using the same function and test name
to keep the expectation precise.
Source: Linters/SAST tools
| def test_generator_profile_in_schema(): | ||
| schema = controls.controls_schema() | ||
| ids = [c["id"] for c in schema] if isinstance(schema, list) else list(schema) | ||
| assert "generator_profile" in ids or "generator_profile" in str(schema) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check the controls payload directly instead of string-matching it.
This currently proves only that the substring appears somewhere in the serialized schema. It does not verify that generator_profile is actually exposed as a control entry in the controls list returned by controls_schema().
Proposed fix
def test_generator_profile_in_schema():
schema = controls.controls_schema()
- ids = [c["id"] for c in schema] if isinstance(schema, list) else list(schema)
- assert "generator_profile" in ids or "generator_profile" in str(schema)
+ assert any(c["id"] == "generator_profile" for c in schema["controls"])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_generator_profile_in_schema(): | |
| schema = controls.controls_schema() | |
| ids = [c["id"] for c in schema] if isinstance(schema, list) else list(schema) | |
| assert "generator_profile" in ids or "generator_profile" in str(schema) | |
| def test_generator_profile_in_schema(): | |
| schema = controls.controls_schema() | |
| assert any(c["id"] == "generator_profile" for c in schema["controls"]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_generator_profile_control.py` around lines 21 - 24, The test in
test_generator_profile_in_schema is too weak because it string-matches the
schema instead of verifying the actual controls payload. Update the assertion to
inspect the structured result from controls.controls_schema() directly and
confirm generator_profile appears as a real control entry in the returned
controls list, using the existing controls_schema symbol and control item ids
rather than str(schema).
| assert model != "default", ( | ||
| f"model resolved to sentinel 'default'; expected a profile-derived model. " | ||
| f"This indicates process_conversation_turn is still using get_memory_model() " | ||
| f"instead of resolve_memory_model()." | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stray f prefixes in this assertion message.
Ruff is flagging Lines 100-102 with F541 because these literals don't interpolate anything, so this file will fail lint as written.
Minimal fix
assert model != "default", (
- f"model resolved to sentinel 'default'; expected a profile-derived model. "
- f"This indicates process_conversation_turn is still using get_memory_model() "
- f"instead of resolve_memory_model()."
+ "model resolved to sentinel 'default'; expected a profile-derived model. "
+ "This indicates process_conversation_turn is still using get_memory_model() "
+ "instead of resolve_memory_model()."
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert model != "default", ( | |
| f"model resolved to sentinel 'default'; expected a profile-derived model. " | |
| f"This indicates process_conversation_turn is still using get_memory_model() " | |
| f"instead of resolve_memory_model()." | |
| ) | |
| assert model != "default", ( | |
| "model resolved to sentinel 'default'; expected a profile-derived model. " | |
| "This indicates process_conversation_turn is still using get_memory_model() " | |
| "instead of resolve_memory_model()." | |
| ) |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 100-100: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 101-101: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 102-102: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_memory_extractor_model_resolution.py` around lines 99 - 103, The
assertion message in the memory model resolution test contains unnecessary
f-string prefixes, which triggers lint error F541. Update the assertion in
tests/test_memory_extractor_model_resolution.py so the multi-line message used
by the assert on model != "default" is plain string text rather than f-strings;
keep the wording intact and remove the stray f prefixes from that assertion
message.
Source: Linters/SAST tools
…rd-settability (#178) merged; bus-auth held for #1507
1 participant
What
Make the answer/memory generator selectable by workload instead of a single global choice. The generator win is task-dependent (gemma4:12b wins LongMemEval single-fact QA but loses LoCoMo and BEAM), so flipping the global default would trade two benchmarks for one. This adds a data-driven, tier-aware generator-profile registry with a safe default.
Design
An orthogonal
taosmd/generator_profiles.pyregistry sits alongside the retrieval recipes. Each profile maps a workload to a generator model per hardware tier (an empty string means retrieval-only on tiny devices).resolve_generatorprecedence is pin > active profile (per-agent > global, defaultbalanced) > recipe generator > retrieval-only.config.resolve_memory_modeldelegates to it, andapply_recipeno longer auto-seedsmemory_model(so a profile is never shadowed by what looked like a user pin).Seeds:
balanced(default): qwen3.5:9b at 12/8 GB, llama3.1:8b at 4 GB, retrieval-only on Pi. Mirrors the previous per-tier recipe generators exactly, so default behaviour is unchanged.factual-recall(opt-in): gemma4:12b at 12 GB, llama3.1:8b at 8 and 4 GB. Wins single-fact retrieval QA; loses on conversational and long-context, so it is opt-in.Backends are
localandnoneonly. Remote and cloud generation are deferred to a follow-up generator-backend-abstraction spec.Evidence
The 8 GB and 4 GB factual picks (llama3.1:8b) are confirmed by the E-023 low-tier bench (F-015): on LongMemEval full-500, llama3.1:8b scored 49.2 (Qwen judge) / 54.4 (llama judge) and beat the shipped qwen3.5:9b (42.8) on the cross-family judge. qwen3:4b was recorded invalid (it leaked the self-verify scratchpad as its answer) and gemma4:e4b was weaker.
Surfacing
taosmd generator-profile list | show <id> | set <id> [--agent NAME]Quality
Built as 8 TDD tasks, each with a spec + quality review, plus a whole-branch review. The whole-branch review caught a real regression: one of three
get_memory_modelread sites (memory_extractor.py) was orphaned by the auto-seed removal, which would have silently dropped LLM fact-extraction to regex on a fresh install. Fixed, with a regression test proven to fail against the buggy code. Full suite 1009 passed.Spec: docs/superpowers/specs/2026-06-24-task-aware-generator-profiles-design.md
Plan: docs/superpowers/plans/2026-06-29-task-aware-generator-profiles.md
Summary by CodeRabbit
New Features
Bug Fixes