Improve council agent UX and persistence#3393
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements a complete Model Council feature enabling multi-round AI agent deliberation. The backend exposes persistent council configurations via new RPC endpoints, orchestrates debate rounds with read-only juror agents, and synthesizes judge conclusions. The frontend provides council list/edit/run management with token estimation and markdown rendering, accompanied by comprehensive test coverage and multilingual translations. ChangesModel Council Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/lib/i18n/ar.ts (1)
4484-4602:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTranslate newly added
modelCouncil.*strings in the Arabic localeSeveral new keys in this Arabic file are still English (for example at Line 4484, Line 4486, Line 4497, Line 4523, Line 4554, Line 4564, Line 4596). This will surface mixed-language UI in Arabic and breaks locale completeness for user-facing text.
As per coding guidelines: “Ensure all i18n locale files have real translations for every key in all 13 supported locales (ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN)”.
🤖 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 `@app/src/lib/i18n/ar.ts` around lines 4484 - 4602, Several modelCouncil.* entries in the Arabic locale remain in English (e.g., keys like modelCouncil.settingsTitle, modelCouncil.debateRoundsLabel, modelCouncil.chairHelp, modelCouncil.run, modelCouncil.runningHint, modelCouncil.usageEstimated) and must be replaced with proper Arabic translations; update each untranslated key in app/src/lib/i18n/ar.ts by providing accurate Arabic strings for those modelCouncil.* keys so the UI is fully localized across Arabic, ensuring phrasing matches existing locale style and preserving placeholders like {count}, {judge}, {round}, {max}, and {model} exactly as in the keys.app/src/lib/i18n/pl.ts (1)
4615-4733:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUntranslated English strings were added to the Polish locale block
Several newly added
modelCouncil.*entries remain in English (for example around Line 4615, Line 4617, Line 4628, Line 4696, Line 4729). Since these keys exist inpl.ts, Polish users will see mixed-language UI instead of a consistent localized experience.Please translate these new values to Polish (or intentionally remove keys to rely on fallback behavior consistently, if that is the project policy for this feature block).
As per coding guidelines: Ensure all i18n locale files have real translations for every key in all 13 supported locales.
🤖 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 `@app/src/lib/i18n/pl.ts` around lines 4615 - 4733, Several modelCouncil.* entries in pl.ts (e.g., modelCouncil.listIntro, modelCouncil.modelPickerHelp, modelCouncil.modelPickerHints, modelCouncil.chairHelp, modelCouncil.judgeWaitingThought, modelCouncil.judgeSynthesizingThought, modelCouncil.scratchpadNoResponse, etc.) are still in English; update each English value to its Polish translation (or remove the key to rely on fallback if that’s the chosen policy) so the Polish locale contains complete translations for all modelCouncil.* keys; search for the modelCouncil.* symbols in the diff to locate every untranslated string, replace the English text with appropriate Polish phrases, and keep interpolation placeholders like {name}, {count}, {round}, {max}, and {judge} intact.
🧹 Nitpick comments (3)
app/src/components/intelligence/ModelCouncilTab.tsx (2)
206-224: ⚡ Quick winUse
async/awaitin effects instead of.then/.catchchains.The provider/model loading effects currently chain promises. This conflicts with the project rule and makes cancellation/error paths harder to read.
🔧 Suggested pattern
useEffect(() => { let active = true; - setProvidersLoading(true); - setProvidersError(null); - loadConnectedModelProviders() - .then(loaded => { - if (!active) return; - ... - }) - .catch(err => { - if (!active) return; - ... - }); + const loadProviders = async () => { + setProvidersLoading(true); + setProvidersError(null); + try { + const loaded = await loadConnectedModelProviders(); + if (!active) return; + ... + } catch (err) { + if (!active) return; + ... + } + }; + void loadProviders(); return () => { active = false; }; }, []);As per coding guidelines:
Always use async/await for promises instead of .then() chains.Also applies to: 256-270
🤖 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 `@app/src/components/intelligence/ModelCouncilTab.tsx` around lines 206 - 224, Convert the promise chains in the useEffect that calls loadConnectedModelProviders to an async function using try/catch/finally: create an inner async function (e.g., fetchProviders) that awaits loadConnectedModelProviders(), checks the local active flag before calling setProviders, setProvider, setProvidersLoading and in catch setProvidersError, and use finally to clear loading; ensure the existing cleanup toggles active to false. Apply the same refactor to the other effect with the same pattern (the block that currently uses .then/.catch around model/provider loading).
1-1870: 🏗️ Heavy liftSplit
ModelCouncilTab.tsxinto smaller modules before it grows further.At ~1870 lines, this file is far past the repository’s size target and now mixes API orchestration, state machine logic, and multiple UI surfaces in one module. Extracting picker dialog, roster editor, deliberation stream panel, and run controller hook would materially improve maintainability.
As per coding guidelines:
Keep TypeScript/React files at or below ~500 lines; split growing modules into smaller files.🤖 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 `@app/src/components/intelligence/ModelCouncilTab.tsx` around lines 1 - 1870, Large single-file component: ModelCouncilTab.tsx has grown past target size and mixes UI, orchestration, and state logic; split into focused modules. Extract ModelPickerDialog and its helpers (parseProviderModel, loadConnectedModelProviders) into a ModelPicker component file; move roster editing UI and seat-related helpers (CouncilSeat types, resolveSeat, profileModel, profileLabel, councilSeatsFromDefinition, SEAT_* constants) into a RosterEditor component; move deliberation/run UI and live state rendering into a DeliberationPanel component; extract orchestration and token/IO logic (handleRun, buildCouncilDefinition/createDraftCouncil, buildCouncilQuestion, buildDebateTurnQuestion, appendScratchpadRound, buildMemberSynthesisInput, estimateTokens, usageEstimate state) into a useModelCouncilController hook file that exposes run/stop, liveMembers, liveScratchpad, usageEstimate, result, and error; keep only top-level composition, view switching, and small glue state in ModelCouncilTab, update imports/props accordingly, and ensure exported symbols like ModelCouncilTab still default-export the composed component.app/src/components/intelligence/ModelCouncilTab.test.tsx (1)
521-541: ⚡ Quick winReduce coupling to internal call ordering in this test.
This section asserts the full 9-call model sequence and specific call indices, which makes the test brittle to harmless orchestration refactors. Prefer asserting user-visible outcomes plus minimal RPC contract expectations (e.g., expected models appear, total rounds count, synthesis invoked).
As per coding guidelines:
Prefer behavior over implementation details in unit tests.🤖 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 `@app/src/components/intelligence/ModelCouncilTab.test.tsx` around lines 521 - 541, The test currently couples to internal call ordering by asserting the exact 9-call model sequence and specific call indices; change it to assert behavior and minimal RPC contracts instead: remove the exact array equality on mockAnswerMember.mock.calls.map(...), and instead assert that the set of models used includes 'critic-model' and 'reasoning-v1' (e.g., check membership or use expect.arrayContaining on the mapped models) and that at least one call uses each model; replace indexed question assertions with content-based checks using any-call predicates (e.g., expect(mockAnswerMember.mock.calls.some(c => c[0].question.includes('User question:'))).toBe(true) and expect(...some(...)...).toContain('What is the capital of France?') and expect(...some(...)...).toContain('Debate round 3 of 3')); keep the minimal RPC expectation that mockSynthesizeCouncil was called with chair_model: 'reasoning-v1' and members: expect.any(Array). This reduces coupling while preserving user-visible behavior and contract checks for mockAnswerMember and mockSynthesizeCouncil.
🤖 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 `@app/src/components/intelligence/ModelCouncilTab.tsx`:
- Around line 87-93: Replace hard-coded user-visible strings with i18n keys via
useT(): for MODEL_HINTS replace the label values
('Default','Chat','Reasoning','Code','Summarize') with useT() calls (e.g.
t('modelHints.default')) and update the constant to compute labels at render
time if needed; similarly replace the seat names/briefs (the array/object around
the component methods that render council seats) and the literals "New council"
and "Chief Judge" with t('...') keys referenced where those strings are used;
add corresponding keys to your translation JSONs and ensure the component
imports and calls useT() to retrieve the translated strings.
In `@app/src/lib/i18n/bn.ts`:
- Around line 4605-4606: The Bengali copy uses an inconsistent/typo
transliteration "জুরর" in the new council strings; update all affected keys
(e.g., modelCouncil.settingsSummary, modelCouncil.juryCountLabel and any other
modelCouncil.* entries around the same block such as the one flagged at 4622) to
use a single correct term (replace "জুরর" with the normalized word you choose)
so spelling is consistent across all new council-related labels; ensure you
update both singular/plural forms in settingsSummary template and the
juryCountLabel key to the same normalized term.
- Around line 4568-4596: The Bengali locale file contains untranslated English
literals under the modelCouncil namespace (e.g., 'modelCouncil.listTitle',
'modelCouncil.listIntro', 'modelCouncil.addCouncil', 'modelCouncil.openCouncil',
'modelCouncil.editCouncil', 'modelCouncil.editCurrentCouncil',
'modelCouncil.editCouncilAria', 'modelCouncil.deleteCouncilAria',
'modelCouncil.backToCouncils', 'modelCouncil.saveCouncil',
'modelCouncil.savingCouncil', 'modelCouncil.cancelEdit',
'modelCouncil.loadingCouncils', 'modelCouncil.noCouncils',
'modelCouncil.noCouncilDescription', 'modelCouncil.registryErrorPrefix',
'modelCouncil.councilNameLabel', 'modelCouncil.councilDescriptionLabel',
'modelCouncil.councilDescriptionPlaceholder', 'modelCouncil.selectModel',
'modelCouncil.modelPickerHelp', 'modelCouncil.closeModelPicker',
'modelCouncil.modelPickerHints', 'modelCouncil.modelPickerProviderModel',
'modelCouncil.modelProviderLabel', 'modelCouncil.modelIdLabel',
'modelCouncil.useProviderModel', 'modelCouncil.questionLabel'); replace each
English string in app/src/lib/i18n/bn.ts for these keys with appropriate Bengali
translations (keeping placeholders like {name} intact), ensuring the
translations cover all modelCouncil entries mentioned in the comment and follow
existing i18n style; then run the locale sanity check or tests to confirm no
missing translations remain.
In `@app/src/lib/i18n/ru.ts`:
- Around line 4615-4642: The new modelCouncil.* entries (e.g.,
'modelCouncil.listTitle', 'modelCouncil.listIntro', 'modelCouncil.addCouncil',
'modelCouncil.openCouncil', 'modelCouncil.editCouncil',
'modelCouncil.editCurrentCouncil', 'modelCouncil.editCouncilAria',
'modelCouncil.deleteCouncilAria', 'modelCouncil.backToCouncils',
'modelCouncil.saveCouncil', 'modelCouncil.savingCouncil',
'modelCouncil.cancelEdit', 'modelCouncil.loadingCouncils',
'modelCouncil.noCouncils', 'modelCouncil.noCouncilDescription',
'modelCouncil.registryErrorPrefix', 'modelCouncil.councilNameLabel',
'modelCouncil.councilDescriptionLabel',
'modelCouncil.councilDescriptionPlaceholder', 'modelCouncil.selectModel',
'modelCouncil.modelPickerHelp', 'modelCouncil.closeModelPicker',
'modelCouncil.modelPickerHints', 'modelCouncil.modelPickerProviderModel',
'modelCouncil.modelProviderLabel', 'modelCouncil.modelIdLabel',
'modelCouncil.useProviderModel') are still in English—replace each value with
the appropriate Russian translation so the ru.ts locale contains all Russian
text (not English) for these keys, keeping the key names unchanged and
preserving punctuation/format placeholders like {name} and ellipses; update any
other modelCouncil.* entries flagged (lines referenced in the review) to Russian
as well to ensure no mixed-language UI.
In `@app/src/services/api/councilRegistryApi.ts`:
- Around line 61-63: The debug logs in the Council registry API are printing raw
user-controlled council identifiers/names (see the log calls inside the get
method and other methods that log council id/name before/after callCoreRpc);
change those log statements to redact or truncate sensitive fields (e.g.,
replace the full id/name with a hashed value, a fixed-length prefix/suffix, or
the string "[REDACTED]") and retain only non-PII context (e.g., operation name
and redacted identifier) so that functions like get and any other methods in
councilRegistryApi.ts no longer emit full user-provided council fields in
diagnostics.
In `@app/src/services/api/modelCouncilApi.ts`:
- Around line 88-89: Replace any logs that include params.question content
(e.g., the log call containing "answer member question=%s model=%s") so they do
not emit raw prompt text; instead log only metadata such as question length
(params.question.length), model name (params.model) and any non-sensitive IDs
(requestId/userId) surrounding the callCoreRpc invocation. Update every matching
log site (the log call near the callCoreRpc and the other occurrences
referenced) to remove slicing/preview of params.question and ensure no PII or
prompt text is written to logs.
In `@scripts/seed-councils.mjs`:
- Around line 8-9: The fallback CORE_BIN constant in scripts/seed-councils.mjs
assumes a Unix binary name and will fail on Windows; update the logic that sets
CORE_BIN (and the equivalent occurrences around the other fallback at lines
~86-91) to detect process.platform === 'win32' and append '.exe' to the resolved
filename when running on Windows so the existence check uses the correct
Windows-safe path (modify the constant initialization for CORE_BIN and the other
fallback variable(s) accordingly).
In `@src/openhuman/council_registry/store.rs`:
- Around line 72-79: The delete_council function currently calls save_store
unconditionally which causes a no-op delete to create an empty councils.json;
change delete_council (in src/openhuman/council_registry/store.rs) to compute
the deleted boolean as now, then only call save_store(&path, &store)? when
deleted is true, and return Ok(RpcOutcome::single_log(deleted, "council registry
deleted")) without writing the file when nothing was removed; ensure the
existing logic that treats (!path.exists() && id == DEFAULT_COUNCIL_ID) is
preserved in the deleted calculation.
In `@tests/json_rpc_e2e.rs`:
- Around line 1819-1827: The assertion that compares captured_models to a
fixed-ordered vec is order-sensitive and flaky because member seats execute
concurrently; change the assertion to be order-insensitive by comparing
multisets instead (e.g., sort or count elements) so the test only verifies that
the four expected model names
["e2e-mock-model","e2e-mock-model","critic-model","e2e-mock-model"] are present
with correct multiplicity; update the assertion around captured_models (the
assert_eq call) to sort both the actual captured_models and the expected vector
(or build frequency maps) before asserting equality.
---
Outside diff comments:
In `@app/src/lib/i18n/ar.ts`:
- Around line 4484-4602: Several modelCouncil.* entries in the Arabic locale
remain in English (e.g., keys like modelCouncil.settingsTitle,
modelCouncil.debateRoundsLabel, modelCouncil.chairHelp, modelCouncil.run,
modelCouncil.runningHint, modelCouncil.usageEstimated) and must be replaced with
proper Arabic translations; update each untranslated key in
app/src/lib/i18n/ar.ts by providing accurate Arabic strings for those
modelCouncil.* keys so the UI is fully localized across Arabic, ensuring
phrasing matches existing locale style and preserving placeholders like {count},
{judge}, {round}, {max}, and {model} exactly as in the keys.
In `@app/src/lib/i18n/pl.ts`:
- Around line 4615-4733: Several modelCouncil.* entries in pl.ts (e.g.,
modelCouncil.listIntro, modelCouncil.modelPickerHelp,
modelCouncil.modelPickerHints, modelCouncil.chairHelp,
modelCouncil.judgeWaitingThought, modelCouncil.judgeSynthesizingThought,
modelCouncil.scratchpadNoResponse, etc.) are still in English; update each
English value to its Polish translation (or remove the key to rely on fallback
if that’s the chosen policy) so the Polish locale contains complete translations
for all modelCouncil.* keys; search for the modelCouncil.* symbols in the diff
to locate every untranslated string, replace the English text with appropriate
Polish phrases, and keep interpolation placeholders like {name}, {count},
{round}, {max}, and {judge} intact.
---
Nitpick comments:
In `@app/src/components/intelligence/ModelCouncilTab.test.tsx`:
- Around line 521-541: The test currently couples to internal call ordering by
asserting the exact 9-call model sequence and specific call indices; change it
to assert behavior and minimal RPC contracts instead: remove the exact array
equality on mockAnswerMember.mock.calls.map(...), and instead assert that the
set of models used includes 'critic-model' and 'reasoning-v1' (e.g., check
membership or use expect.arrayContaining on the mapped models) and that at least
one call uses each model; replace indexed question assertions with content-based
checks using any-call predicates (e.g.,
expect(mockAnswerMember.mock.calls.some(c => c[0].question.includes('User
question:'))).toBe(true) and expect(...some(...)...).toContain('What is the
capital of France?') and expect(...some(...)...).toContain('Debate round 3 of
3')); keep the minimal RPC expectation that mockSynthesizeCouncil was called
with chair_model: 'reasoning-v1' and members: expect.any(Array). This reduces
coupling while preserving user-visible behavior and contract checks for
mockAnswerMember and mockSynthesizeCouncil.
In `@app/src/components/intelligence/ModelCouncilTab.tsx`:
- Around line 206-224: Convert the promise chains in the useEffect that calls
loadConnectedModelProviders to an async function using try/catch/finally: create
an inner async function (e.g., fetchProviders) that awaits
loadConnectedModelProviders(), checks the local active flag before calling
setProviders, setProvider, setProvidersLoading and in catch setProvidersError,
and use finally to clear loading; ensure the existing cleanup toggles active to
false. Apply the same refactor to the other effect with the same pattern (the
block that currently uses .then/.catch around model/provider loading).
- Around line 1-1870: Large single-file component: ModelCouncilTab.tsx has grown
past target size and mixes UI, orchestration, and state logic; split into
focused modules. Extract ModelPickerDialog and its helpers (parseProviderModel,
loadConnectedModelProviders) into a ModelPicker component file; move roster
editing UI and seat-related helpers (CouncilSeat types, resolveSeat,
profileModel, profileLabel, councilSeatsFromDefinition, SEAT_* constants) into a
RosterEditor component; move deliberation/run UI and live state rendering into a
DeliberationPanel component; extract orchestration and token/IO logic
(handleRun, buildCouncilDefinition/createDraftCouncil, buildCouncilQuestion,
buildDebateTurnQuestion, appendScratchpadRound, buildMemberSynthesisInput,
estimateTokens, usageEstimate state) into a useModelCouncilController hook file
that exposes run/stop, liveMembers, liveScratchpad, usageEstimate, result, and
error; keep only top-level composition, view switching, and small glue state in
ModelCouncilTab, update imports/props accordingly, and ensure exported symbols
like ModelCouncilTab still default-export the composed component.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41ec134a-3612-450a-b094-157eb5ae1623
📒 Files selected for processing (32)
app/src/components/intelligence/ModelCouncilTab.test.tsxapp/src/components/intelligence/ModelCouncilTab.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/services/api/councilRegistryApi.test.tsapp/src/services/api/councilRegistryApi.tsapp/src/services/api/modelCouncilApi.test.tsapp/src/services/api/modelCouncilApi.tspackage.jsonscripts/seed-councils.mjssrc/core/all.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/council_registry/mod.rssrc/openhuman/council_registry/schemas.rssrc/openhuman/council_registry/store.rssrc/openhuman/council_registry/types.rssrc/openhuman/mod.rssrc/openhuman/model_council/council.rssrc/openhuman/model_council/schemas.rstests/json_rpc_e2e.rs
Comment on lines
+87
to
+93
| const MODEL_HINTS = [ | ||
| { value: 'default', label: 'Default' }, | ||
| { value: 'hint:chat', label: 'Chat' }, | ||
| { value: 'hint:reasoning', label: 'Reasoning' }, | ||
| { value: 'hint:code', label: 'Code' }, | ||
| { value: 'hint:summarize', label: 'Summarize' }, | ||
| ] as const; |
Contributor
There was a problem hiding this comment.
Move user-visible default labels/copy to useT() translation keys.
Line 88 (Default/Chat/Reasoning/...), Line 418+ (seat names/briefs), and Line 651+ (New council, Chief Judge) are directly user-visible English literals. These should come from i18n keys.
As per coding guidelines: Every user-visible string in app/src/** ... must go through useT().
Also applies to: 407-437, 651-663
🤖 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 `@app/src/components/intelligence/ModelCouncilTab.tsx` around lines 87 - 93,
Replace hard-coded user-visible strings with i18n keys via useT(): for
MODEL_HINTS replace the label values
('Default','Chat','Reasoning','Code','Summarize') with useT() calls (e.g.
t('modelHints.default')) and update the constant to compute labels at render
time if needed; similarly replace the seat names/briefs (the array/object around
the component methods that render council seats) and the literals "New council"
and "Chief Judge" with t('...') keys referenced where those strings are used;
add corresponding keys to your translation JSONs and ensure the component
imports and calls useT() to retrieve the translated strings.
Comment on lines
+4568
to
4596
| 'modelCouncil.listTitle': 'Councils', | ||
| 'modelCouncil.listIntro': | ||
| 'Choose a saved council, edit its agents and judge, or create a new council for a specific kind of decision.', | ||
| 'modelCouncil.addCouncil': 'New council', | ||
| 'modelCouncil.openCouncil': 'Open council', | ||
| 'modelCouncil.editCouncil': 'Edit council', | ||
| 'modelCouncil.editCurrentCouncil': 'Edit current council', | ||
| 'modelCouncil.editCouncilAria': 'Edit {name}', | ||
| 'modelCouncil.deleteCouncilAria': 'Delete {name}', | ||
| 'modelCouncil.backToCouncils': 'Back to councils', | ||
| 'modelCouncil.saveCouncil': 'Save council', | ||
| 'modelCouncil.savingCouncil': 'Saving…', | ||
| 'modelCouncil.cancelEdit': 'Cancel', | ||
| 'modelCouncil.loadingCouncils': 'Loading councils…', | ||
| 'modelCouncil.noCouncils': 'No councils yet. Add one to get started.', | ||
| 'modelCouncil.noCouncilDescription': 'No description', | ||
| 'modelCouncil.registryErrorPrefix': 'Council registry failed:', | ||
| 'modelCouncil.councilNameLabel': 'Council name', | ||
| 'modelCouncil.councilDescriptionLabel': 'Description', | ||
| 'modelCouncil.councilDescriptionPlaceholder': 'When to use this council', | ||
| 'modelCouncil.selectModel': 'Select', | ||
| 'modelCouncil.modelPickerHelp': 'Select a routing hint or pin an exact provider model.', | ||
| 'modelCouncil.closeModelPicker': 'Close', | ||
| 'modelCouncil.modelPickerHints': 'Hints', | ||
| 'modelCouncil.modelPickerProviderModel': 'Provider + model', | ||
| 'modelCouncil.modelProviderLabel': 'Model provider', | ||
| 'modelCouncil.modelIdLabel': 'Model id', | ||
| 'modelCouncil.useProviderModel': 'Use provider model', | ||
| 'modelCouncil.questionLabel': 'Question', |
Contributor
There was a problem hiding this comment.
Translate newly added modelCouncil Bengali strings instead of shipping English literals.
Several new Bengali locale entries are still English (for example around Line 4568, Line 4571, Line 4581, Line 4648, Line 4680). This causes mixed-language UI in the Bengali locale for the new council flows.
As per coding guidelines: “Ensure all i18n locale files have real translations for every key in all 13 supported locales (ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN)”.
Also applies to: 4648-4686
🤖 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 `@app/src/lib/i18n/bn.ts` around lines 4568 - 4596, The Bengali locale file
contains untranslated English literals under the modelCouncil namespace (e.g.,
'modelCouncil.listTitle', 'modelCouncil.listIntro', 'modelCouncil.addCouncil',
'modelCouncil.openCouncil', 'modelCouncil.editCouncil',
'modelCouncil.editCurrentCouncil', 'modelCouncil.editCouncilAria',
'modelCouncil.deleteCouncilAria', 'modelCouncil.backToCouncils',
'modelCouncil.saveCouncil', 'modelCouncil.savingCouncil',
'modelCouncil.cancelEdit', 'modelCouncil.loadingCouncils',
'modelCouncil.noCouncils', 'modelCouncil.noCouncilDescription',
'modelCouncil.registryErrorPrefix', 'modelCouncil.councilNameLabel',
'modelCouncil.councilDescriptionLabel',
'modelCouncil.councilDescriptionPlaceholder', 'modelCouncil.selectModel',
'modelCouncil.modelPickerHelp', 'modelCouncil.closeModelPicker',
'modelCouncil.modelPickerHints', 'modelCouncil.modelPickerProviderModel',
'modelCouncil.modelProviderLabel', 'modelCouncil.modelIdLabel',
'modelCouncil.useProviderModel', 'modelCouncil.questionLabel'); replace each
English string in app/src/lib/i18n/bn.ts for these keys with appropriate Bengali
translations (keeping placeholders like {name} intact), ensuring the
translations cover all modelCouncil entries mentioned in the comment and follow
existing i18n style; then run the locale sanity check or tests to confirm no
missing translations remain.
Comment on lines
+4605
to
+4606
| 'modelCouncil.settingsSummary': '{count} জন জুরর · বিচারক: {judge}', | ||
| 'modelCouncil.juryCountLabel': 'জুরর সংখ্যা', |
Contributor
There was a problem hiding this comment.
Fix juror label spelling consistency in Bengali copy.
জুরর appears in multiple new labels and looks like a typo/inconsistent transliteration. Please normalize this term consistently across the new council strings.
Proposed copy fix
- 'modelCouncil.settingsSummary': '{count} জন জুরর · বিচারক: {judge}',
- 'modelCouncil.juryCountLabel': 'জুরর সংখ্যা',
+ 'modelCouncil.settingsSummary': '{count} জন জুরার · বিচারক: {judge}',
+ 'modelCouncil.juryCountLabel': 'জুরার সংখ্যা',
...
- 'modelCouncil.jurorLabel': 'জুরর {n}',
+ 'modelCouncil.jurorLabel': 'জুরার {n}',Also applies to: 4622-4622
🤖 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 `@app/src/lib/i18n/bn.ts` around lines 4605 - 4606, The Bengali copy uses an
inconsistent/typo transliteration "জুরর" in the new council strings; update all
affected keys (e.g., modelCouncil.settingsSummary, modelCouncil.juryCountLabel
and any other modelCouncil.* entries around the same block such as the one
flagged at 4622) to use a single correct term (replace "জুরর" with the
normalized word you choose) so spelling is consistent across all new
council-related labels; ensure you update both singular/plural forms in
settingsSummary template and the juryCountLabel key to the same normalized term.
Comment on lines
+4615
to
+4642
| 'modelCouncil.listTitle': 'Councils', | ||
| 'modelCouncil.listIntro': | ||
| 'Choose a saved council, edit its agents and judge, or create a new council for a specific kind of decision.', | ||
| 'modelCouncil.addCouncil': 'New council', | ||
| 'modelCouncil.openCouncil': 'Open council', | ||
| 'modelCouncil.editCouncil': 'Edit council', | ||
| 'modelCouncil.editCurrentCouncil': 'Edit current council', | ||
| 'modelCouncil.editCouncilAria': 'Edit {name}', | ||
| 'modelCouncil.deleteCouncilAria': 'Delete {name}', | ||
| 'modelCouncil.backToCouncils': 'Back to councils', | ||
| 'modelCouncil.saveCouncil': 'Save council', | ||
| 'modelCouncil.savingCouncil': 'Saving…', | ||
| 'modelCouncil.cancelEdit': 'Cancel', | ||
| 'modelCouncil.loadingCouncils': 'Loading councils…', | ||
| 'modelCouncil.noCouncils': 'No councils yet. Add one to get started.', | ||
| 'modelCouncil.noCouncilDescription': 'No description', | ||
| 'modelCouncil.registryErrorPrefix': 'Council registry failed:', | ||
| 'modelCouncil.councilNameLabel': 'Council name', | ||
| 'modelCouncil.councilDescriptionLabel': 'Description', | ||
| 'modelCouncil.councilDescriptionPlaceholder': 'When to use this council', | ||
| 'modelCouncil.selectModel': 'Select', | ||
| 'modelCouncil.modelPickerHelp': 'Select a routing hint or pin an exact provider model.', | ||
| 'modelCouncil.closeModelPicker': 'Close', | ||
| 'modelCouncil.modelPickerHints': 'Hints', | ||
| 'modelCouncil.modelPickerProviderModel': 'Provider + model', | ||
| 'modelCouncil.modelProviderLabel': 'Model provider', | ||
| 'modelCouncil.modelIdLabel': 'Model id', | ||
| 'modelCouncil.useProviderModel': 'Use provider model', |
Contributor
There was a problem hiding this comment.
Replace newly added English modelCouncil.* values with Russian translations.
Several new Russian-locale entries are still English (for example at Line 4615, Line 4654, Line 4695, Line 4727). This will ship mixed-language UI in the council experience.
As per coding guidelines, app/src/lib/i18n/*.ts: “Ensure all i18n locale files have real translations for every key in all 13 supported locales (ar, bn, de, es, fr, hi, id, it, ko, pl, pt, ru, zh-CN)”.
Also applies to: 4654-4656, 4685-4688, 4695-4733
🤖 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 `@app/src/lib/i18n/ru.ts` around lines 4615 - 4642, The new modelCouncil.*
entries (e.g., 'modelCouncil.listTitle', 'modelCouncil.listIntro',
'modelCouncil.addCouncil', 'modelCouncil.openCouncil',
'modelCouncil.editCouncil', 'modelCouncil.editCurrentCouncil',
'modelCouncil.editCouncilAria', 'modelCouncil.deleteCouncilAria',
'modelCouncil.backToCouncils', 'modelCouncil.saveCouncil',
'modelCouncil.savingCouncil', 'modelCouncil.cancelEdit',
'modelCouncil.loadingCouncils', 'modelCouncil.noCouncils',
'modelCouncil.noCouncilDescription', 'modelCouncil.registryErrorPrefix',
'modelCouncil.councilNameLabel', 'modelCouncil.councilDescriptionLabel',
'modelCouncil.councilDescriptionPlaceholder', 'modelCouncil.selectModel',
'modelCouncil.modelPickerHelp', 'modelCouncil.closeModelPicker',
'modelCouncil.modelPickerHints', 'modelCouncil.modelPickerProviderModel',
'modelCouncil.modelProviderLabel', 'modelCouncil.modelIdLabel',
'modelCouncil.useProviderModel') are still in English—replace each value with
the appropriate Russian translation so the ru.ts locale contains all Russian
text (not English) for these keys, keeping the key names unchanged and
preserving punctuation/format placeholders like {name} and ellipses; update any
other modelCouncil.* entries flagged (lines referenced in the review) to Russian
as well to ensure no mixed-language UI.
Comment on lines
+61
to
+63
| get: async (id: string): Promise<CouncilDefinition | null> => { | ||
| log('get council id=%s', id); | ||
| const payload = await callCoreRpc<unknown>({ |
Contributor
There was a problem hiding this comment.
Redact user-provided council fields from debug logs.
Line 62, Line 71, and Line 80 log raw council identifiers/names. Those are user-controlled fields and should be redacted/minimized in diagnostics.
🔧 Suggested patch
- log('get council id=%s', id);
+ log('get council');
- log('upsert council id=%s name=%s', council.id, council.name);
+ log(
+ 'upsert council id_present=%s name_len=%d',
+ Boolean(council.id?.trim()),
+ council.name?.length ?? 0
+ );
- log('delete council id=%s', id);
+ log('delete council');As per coding guidelines: Never log secrets or full PII—redact sensitive information in all logs.
Also applies to: 70-72, 79-80
🤖 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 `@app/src/services/api/councilRegistryApi.ts` around lines 61 - 63, The debug
logs in the Council registry API are printing raw user-controlled council
identifiers/names (see the log calls inside the get method and other methods
that log council id/name before/after callCoreRpc); change those log statements
to redact or truncate sensitive fields (e.g., replace the full id/name with a
hashed value, a fixed-length prefix/suffix, or the string "[REDACTED]") and
retain only non-PII context (e.g., operation name and redacted identifier) so
that functions like get and any other methods in councilRegistryApi.ts no longer
emit full user-provided council fields in diagnostics.
Comment on lines
+88
to
+89
| log('answer member question=%s model=%s', params.question.slice(0, 40), params.model); | ||
| const payload = await callCoreRpc<unknown>({ |
Contributor
There was a problem hiding this comment.
Do not log question content; log metadata only.
Line 88, Line 100, and Line 115 include question text in logs. User prompts can contain sensitive data, so diagnostics should avoid raw content.
🔧 Suggested patch
- log('answer member question=%s model=%s', params.question.slice(0, 40), params.model);
+ log('answer member question_len=%d model=%s', params.question.length, params.model);
- 'synthesize question=%s members=%d chair=%s',
- params.question.slice(0, 40),
+ 'synthesize question_len=%d members=%d chair=%s',
+ params.question.length,
params.members.length,
params.chair_model
- 'run question=%s members=%o chair=%s',
- params.question.slice(0, 40),
- params.member_models,
+ 'run question_len=%d members_count=%d chair=%s',
+ params.question.length,
+ params.member_models.length,
params.chair_modelAs per coding guidelines: Never log secrets or full PII—redact sensitive information in all logs.
Also applies to: 99-103, 114-117
🤖 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 `@app/src/services/api/modelCouncilApi.ts` around lines 88 - 89, Replace any
logs that include params.question content (e.g., the log call containing "answer
member question=%s model=%s") so they do not emit raw prompt text; instead log
only metadata such as question length (params.question.length), model name
(params.model) and any non-sensitive IDs (requestId/userId) surrounding the
callCoreRpc invocation. Update every matching log site (the log call near the
callCoreRpc and the other occurrences referenced) to remove slicing/preview of
params.question and ensure no PII or prompt text is written to logs.
Comment on lines
+8
to
+9
| const CORE_BIN = process.env.OPENHUMAN_CORE_BIN || resolve('target/debug/openhuman-core'); | ||
|
|
Contributor
There was a problem hiding this comment.
Default core binary path is not Windows-safe.
The fallback path assumes an extensionless binary, so Windows users will fail the existence check unless they manually override OPENHUMAN_CORE_BIN.
🐛 Proposed fix
-const CORE_BIN = process.env.OPENHUMAN_CORE_BIN || resolve('target/debug/openhuman-core');
+const defaultCoreBin = resolve(
+ 'target',
+ 'debug',
+ process.platform === 'win32' ? 'openhuman-core.exe' : 'openhuman-core'
+);
+const CORE_BIN = process.env.OPENHUMAN_CORE_BIN || defaultCoreBin;Also applies to: 86-91
🤖 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 `@scripts/seed-councils.mjs` around lines 8 - 9, The fallback CORE_BIN constant
in scripts/seed-councils.mjs assumes a Unix binary name and will fail on
Windows; update the logic that sets CORE_BIN (and the equivalent occurrences
around the other fallback at lines ~86-91) to detect process.platform ===
'win32' and append '.exe' to the resolved filename when running on Windows so
the existence check uses the correct Windows-safe path (modify the constant
initialization for CORE_BIN and the other fallback variable(s) accordingly).
Comment on lines
+72
to
+79
| pub fn delete_council(config: &Config, id: &str) -> Result<RpcOutcome<bool>, String> { | ||
| let path = store_path(config); | ||
| let mut store = load_store(&path)?; | ||
| let before = store.councils.len(); | ||
| store.councils.retain(|council| council.id != id); | ||
| let deleted = before != store.councils.len() || (!path.exists() && id == DEFAULT_COUNCIL_ID); | ||
| save_store(&path, &store)?; | ||
| Ok(RpcOutcome::single_log(deleted, "council registry deleted")) |
Contributor
There was a problem hiding this comment.
Prevent no-op deletes from materializing an empty registry file.
On Line 78, save_store runs even when deleted == false. In a fresh workspace, deleting a non-existent non-default ID creates councils.json, which then hides the implicit default council on later list calls.
Suggested fix
pub fn delete_council(config: &Config, id: &str) -> Result<RpcOutcome<bool>, String> {
let path = store_path(config);
let mut store = load_store(&path)?;
let before = store.councils.len();
store.councils.retain(|council| council.id != id);
- let deleted = before != store.councils.len() || (!path.exists() && id == DEFAULT_COUNCIL_ID);
- save_store(&path, &store)?;
+ let removed_from_store = before != store.councils.len();
+ let deleted_implicit_default = !path.exists() && id == DEFAULT_COUNCIL_ID;
+ let deleted = removed_from_store || deleted_implicit_default;
+ if deleted {
+ save_store(&path, &store)?;
+ }
Ok(RpcOutcome::single_log(deleted, "council registry deleted"))
}🤖 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 `@src/openhuman/council_registry/store.rs` around lines 72 - 79, The
delete_council function currently calls save_store unconditionally which causes
a no-op delete to create an empty councils.json; change delete_council (in
src/openhuman/council_registry/store.rs) to compute the deleted boolean as now,
then only call save_store(&path, &store)? when deleted is true, and return
Ok(RpcOutcome::single_log(deleted, "council registry deleted")) without writing
the file when nothing was removed; ensure the existing logic that treats
(!path.exists() && id == DEFAULT_COUNCIL_ID) is preserved in the deleted
calculation.
Comment on lines
+1819
to
+1827
| assert_eq!( | ||
| captured_models, | ||
| vec![ | ||
| "e2e-mock-model", | ||
| "e2e-mock-model", | ||
| "critic-model", | ||
| "e2e-mock-model" | ||
| ], | ||
| "three member calls plus one default chair call should be made" |
Contributor
There was a problem hiding this comment.
Avoid order-sensitive assertions for concurrent member fan-out.
Line 1819 asserts a fixed model-call order even though member seats are executed concurrently, so request arrival order can vary and make this test flaky.
Proposed assertion update
- assert_eq!(
- captured_models,
- vec![
- "e2e-mock-model",
- "e2e-mock-model",
- "critic-model",
- "e2e-mock-model"
- ],
- "three member calls plus one default chair call should be made"
- );
+ assert_eq!(
+ captured_models.len(),
+ 4,
+ "expected three member calls plus one chair call"
+ );
+ assert_eq!(
+ captured_models.last().map(String::as_str),
+ Some("e2e-mock-model"),
+ "chair call should happen after member fan-out completes"
+ );
+ let mut member_models = captured_models[..3].to_vec();
+ member_models.sort();
+ assert_eq!(
+ member_models,
+ vec![
+ "critic-model".to_string(),
+ "e2e-mock-model".to_string(),
+ "e2e-mock-model".to_string()
+ ],
+ "member fan-out should include two defaults and one critic"
+ );🤖 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/json_rpc_e2e.rs` around lines 1819 - 1827, The assertion that compares
captured_models to a fixed-ordered vec is order-sensitive and flaky because
member seats execute concurrently; change the assertion to be order-insensitive
by comparing multisets instead (e.g., sort or count elements) so the test only
verifies that the four expected model names
["e2e-mock-model","e2e-mock-model","critic-model","e2e-mock-model"] are present
with correct multiplicity; update the assertion around captured_models (the
assert_eq call) to sort both the actual captured_models and the expected vector
(or build frequency maps) before asserting equality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reasoning-v1as the default model.pnpm council:seedscript for workspace council presets.Problem
Solution
council_registrycore module exposed through controller registry methods for list/get/upsert/delete.scripts/seed-councils.mjs, wired aspnpm council:seed, which callsopenhuman-corecontroller methods and writes through core persistence.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/pr-ci.yml. Local full coverage was not run; targeted Vitest/Rust coverage was added and CI enforces the gate.## Related— N/A: no matrix feature IDs apply.docs/RELEASE-MANUAL-SMOKE.md) — N/A: not a release-cut smoke checklist surface.Closes #NNNin the## Relatedsection — N/A: no GitHub issue was provided for this work.Impact
Config.workspace_dir/council_registry/councils.json.pnpm council:seedcan seed council presets throughopenhuman-core.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
council-agents-ux8bdde21cdValidation Run
pnpm --filter openhuman-app format:check— passed via pre-push hookpnpm typecheck— passedpnpm debug unit src/components/intelligence/ModelCouncilTab.test.tsx --verbose;pnpm i18n:check --locale zh-CN,hi,es,ar,fr,bn,pt,de,ru,id,it,ko,pl;pnpm council:seed --dry-run; temp-workspacepnpm council:seed --workspace <tmp> --replaceGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml council_registry -q;cargo build --manifest-path Cargo.toml --bin openhuman-core;cargo check --manifest-path Cargo.tomlvia pre-push hookcargo check --manifest-path app/src-tauri/Cargo.tomlvia pre-push hookValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Release Notes