Skip to content

Commit 89131e1

Browse files
committed
fix(studio): address assistant PR checks
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
1 parent 1b9fcd0 commit 89131e1

18 files changed

Lines changed: 195 additions & 46 deletions

File tree

Binary file not shown.

k8s/helm/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ and
401401
| openshiftRoute.service | string | `"{{ include \"nemo-platform.ingressBackendService\" . }}"` | Service name to route to. Defaults to Envoy when auth+envoy enabled, otherwise API (tpl-evaluated). |
402402
| openshiftRoute.targetPort | string | `"{{ include \"nemo-platform.ingressBackendPort\" . }}"` | Target port on the service. Defaults to Envoy or API port depending on auth (tpl-evaluated). |
403403
| openshiftRoute.tls | object | `{}` | Optional TLS configuration (termination, certificate, key, etc.). See OpenShift Route spec. |
404-
| platformConfig | object | `{}` | Platform-wide configuration settings Set configuration here to apply custom, structured configuration across all services. Applied after the base platform config is evaluated for templates. Enables adding / overriding YAML-based elements in the evaluated platform config. It is usually recommended to use this config section instead of `basePlatformConfig` unless you need to use templating features. For example, you can set the NIM default StorageClass via models.controller.backends.deployments_plugin.default_storage_class. For full configuration reference, see https://docs.nvidia.com/nemo-platform |
404+
| platformConfig | object | `{"studio":{"feature_flags":{"assistant_studio_enabled":true}}}` | Platform-wide configuration settings Set configuration here to apply custom, structured configuration across all services. Applied after the base platform config is evaluated for templates. Enables adding / overriding YAML-based elements in the evaluated platform config. It is usually recommended to use this config section instead of `basePlatformConfig` unless you need to use templating features. For example, you can set the NIM default StorageClass via models.controller.backends.deployments_plugin.default_storage_class. For full configuration reference, see https://docs.nvidia.com/nemo-platform |
405405
| platformSeedJob | object | This object has the following default values for the platform seed Job configuration. | Platform seed Job (Helm hook: runs after install/upgrade) Runs the platform-seed task (guardrails configs, evaluator system entities, data designer filesets). Uses post-install,post-upgrade hooks so it runs on fresh installs and can be re-triggered on no-op upgrade. |
406406
| platformSeedJob.activeDeadlineSeconds | int | `600` | Maximum time in seconds the Job can run. |
407407
| platformSeedJob.affinity | object | `{}` | Affinity for the platform seeding Job pod. |

k8s/helm/values.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,10 @@ externalClickhouse:
366366
# It is usually recommended to use this config section instead of `basePlatformConfig` unless you need to use templating features.
367367
# For example, you can set the NIM default StorageClass via models.controller.backends.deployments_plugin.default_storage_class.
368368
# For full configuration reference, see https://docs.nvidia.com/nemo-platform
369-
platformConfig: {}
369+
platformConfig:
370+
studio:
371+
feature_flags:
372+
assistant_studio_enabled: true
370373

371374
# -- Base platform configuration settings
372375
# @default -- This object has the following default values for the base platform configuration.

services/studio/src/nmp/studio/assistant.py

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
permission_prompt_tool,
5555
)
5656
from nmp.studio.assistant_skills import ClaudeSkillResponse, DuplicateSkillError, list_claude_skill_responses
57-
from nmp.studio.entities import AssistantConversation, AssistantMessage
57+
from nmp.studio.entities import AssistantConversation, AssistantMessage, LegacyAssistantConversation
5858
from pydantic import BaseModel, ConfigDict, Field
5959
from starlette.routing import NoMatchFound
6060

@@ -229,6 +229,32 @@ def _conversation_name(session_id: str) -> str:
229229
return f"assistant-{session_id}"
230230

231231

232+
def _legacy_conversation_name(session_id: str) -> str:
233+
"""Return the Entity Store name used before the Assistant rename."""
234+
return f"copilot-{session_id}"
235+
236+
237+
async def _get_conversation(
238+
entity_store: EntityClient,
239+
*,
240+
session_id: str,
241+
workspace: str,
242+
) -> AssistantConversation:
243+
"""Load a current conversation, falling back to its pre-rename identity."""
244+
try:
245+
return await entity_store.get(
246+
AssistantConversation,
247+
_conversation_name(session_id),
248+
workspace=workspace,
249+
)
250+
except EntityNotFoundError:
251+
return await entity_store.get(
252+
LegacyAssistantConversation,
253+
_legacy_conversation_name(session_id),
254+
workspace=workspace,
255+
)
256+
257+
232258
def _request_principal_id(request: Request) -> str:
233259
"""Return the end-user principal, including service-on-behalf-of requests."""
234260
return (
@@ -245,9 +271,9 @@ async def _get_owned_conversation(
245271
) -> AssistantConversation:
246272
"""Load a conversation and enforce per-user ownership within a workspace."""
247273
try:
248-
conversation = await entity_store.get(
249-
AssistantConversation,
250-
_conversation_name(session_id),
274+
conversation = await _get_conversation(
275+
entity_store,
276+
session_id=session_id,
251277
workspace=workspace,
252278
)
253279
except EntityNotFoundError as exc:
@@ -749,15 +775,22 @@ async def list_history_sessions(
749775
"""List the current user's durable NeMo Assistant sessions."""
750776
workspace = _validated_workspace_or_default(workspace)
751777
owner_id = _request_principal_id(request)
752-
result = await entity_store.list(
753-
AssistantConversation,
754-
workspace=workspace,
755-
filter_obj={"owner_id": owner_id},
756-
sort="-updated_at",
757-
page_size=MAX_RETAINED_SESSIONS,
758-
)
778+
results = [
779+
await entity_store.list(
780+
entity_type,
781+
workspace=workspace,
782+
filter_obj={"owner_id": owner_id},
783+
sort="-updated_at",
784+
page_size=MAX_RETAINED_SESSIONS,
785+
)
786+
for entity_type in (AssistantConversation, LegacyAssistantConversation)
787+
]
759788
sessions: list[HistorySessionResponse] = []
760-
for conversation in result.data:
789+
seen_session_ids: set[str] = set()
790+
for conversation in (conversation for result in results for conversation in result.data):
791+
if conversation.session_id in seen_session_ids:
792+
continue
793+
seen_session_ids.add(conversation.session_id)
761794
user_messages = [message.content for message in conversation.messages if message.role == "user"]
762795
if not user_messages:
763796
continue
@@ -865,11 +898,7 @@ async def get_session_history(
865898
sid = _validate_session_id(session_id)
866899
workspace = _validated_workspace_or_default(workspace)
867900
try:
868-
conversation = await entity_store.get(
869-
AssistantConversation,
870-
_conversation_name(sid),
871-
workspace=workspace,
872-
)
901+
conversation = await _get_conversation(entity_store, session_id=sid, workspace=workspace)
873902
except EntityNotFoundError:
874903
conversation = None
875904
if conversation is not None:
@@ -964,7 +993,7 @@ async def delete_session_history(
964993
)
965994
try:
966995
await entity_store.delete(
967-
AssistantConversation,
996+
type(conversation),
968997
conversation.name,
969998
workspace=workspace,
970999
expected_db_version=conversation.db_version,

services/studio/src/nmp/studio/assistant_artifacts.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from typing import Any
1010

1111
from nmp.studio import studio_links
12-
from pydantic import BaseModel, Field
12+
from pydantic import AliasChoices, BaseModel, Field
1313

1414

1515
class ChatSelectionArtifactResponse(BaseModel):
@@ -49,7 +49,10 @@ class ChatArtifactsResponse(BaseModel):
4949
agent: str | None = None
5050
model: str | None = None
5151
model_source: str | None = None
52-
assistant_model: str | None = None
52+
assistant_model: str | None = Field(
53+
default=None,
54+
validation_alias=AliasChoices("assistant_model", "copilot_model"),
55+
)
5356
workspace: str | None = None
5457
selections: list[ChatSelectionArtifactResponse] = Field(default_factory=list)
5558
files: list[ChatFileArtifactResponse] = Field(default_factory=list)

services/studio/src/nmp/studio/entities.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,9 @@ class AssistantConversation(EntityBase):
2626
owner_id: str = Field(description="Principal that owns and may read this conversation.")
2727
messages: list[AssistantMessage] = Field(default_factory=list)
2828
chat_artifacts: ChatArtifactsResponse = Field(default_factory=ChatArtifactsResponse)
29+
30+
31+
class LegacyAssistantConversation(AssistantConversation):
32+
"""Read-compatible model for conversations persisted before the rename."""
33+
34+
__entity_type__: ClassVar[str] = "copilot_conversation"

services/studio/tests/unit/test_assistant.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from nmp.common.service.dependencies import get_entity_client
2424
from nmp.studio import assistant, assistant_artifacts, assistant_skills, studio_links
2525
from nmp.studio.config import StudioConfig
26-
from nmp.studio.entities import AssistantConversation, AssistantMessage
26+
from nmp.studio.entities import AssistantConversation, AssistantMessage, LegacyAssistantConversation
2727
from nmp.studio.service import StudioService
2828

2929

@@ -389,6 +389,46 @@ def test_list_history_sessions_includes_persisted_conversation(
389389
]
390390

391391

392+
def test_legacy_conversation_remains_listable_readable_and_deletable(
393+
service_client: TestClient,
394+
entity_store: FakeEntityStore,
395+
):
396+
session_id = str(uuid.uuid4())
397+
conversation = LegacyAssistantConversation.model_validate(
398+
{
399+
"name": f"copilot-{session_id}",
400+
"workspace": "default",
401+
"session_id": session_id,
402+
"owner_id": "local-user",
403+
"messages": [
404+
{"role": "user", "content": "Legacy prompt"},
405+
{"role": "assistant", "content": "Legacy answer"},
406+
],
407+
"chat_artifacts": {
408+
"model_source": "copilot",
409+
"copilot_model": "nvidia/legacy-model",
410+
},
411+
}
412+
)
413+
conversation._created_at = datetime.fromtimestamp(40, UTC)
414+
conversation._updated_at = datetime.fromtimestamp(42, UTC)
415+
entity_store.entities[("default", conversation.name)] = conversation
416+
417+
list_response = service_client.get("/v2/assistant/history/sessions")
418+
history_response = service_client.get(f"/v2/assistant/history/sessions/{session_id}")
419+
delete_response = service_client.delete(f"/v2/assistant/history/sessions/{session_id}")
420+
421+
assert list_response.status_code == 200
422+
assert list_response.json()[0]["chat_artifacts"]["assistant_model"] == "nvidia/legacy-model"
423+
assert history_response.status_code == 200
424+
assert history_response.json()["items"] == [
425+
{"kind": "user", "text": "Legacy prompt"},
426+
{"kind": "assistant", "parts": [{"type": "text", "text": "Legacy answer"}]},
427+
]
428+
assert delete_response.status_code == 204
429+
assert ("default", conversation.name) not in entity_store.entities
430+
431+
392432
def test_history_is_scoped_to_workspace_and_owner(
393433
service_client: TestClient,
394434
entity_store: FakeEntityStore,

web/.prettierignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ pnpm-lock.yaml
77
# Build output
88
dist/
99
packages/*/dist/
10+
packages/studio/public/vendor/
1011
packages/**/.test-reports/
1112
packages/**/test-results/
1213
packages/**/playwright-report/
1314

1415
# Ignore generated style
15-
packages/studio/src/generated/*
16+
packages/studio/src/generated/*

web/packages/studio/src/routes/agents/AssistantChatRoute/AssistantHistoryPanel.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import { type FC } from 'react';
1717

1818
type OpenFloatingPanel = 'history' | 'skills';
1919

20-
export const AssistantHistoryPanel: FC<AssistantHistoryPanelProps> = ({ hideArtifacts, ...props }) => {
20+
export const AssistantHistoryPanel: FC<AssistantHistoryPanelProps> = ({
21+
hideArtifacts,
22+
...props
23+
}) => {
2124
const [historyOpen, setHistoryOpen] = useLocalStorage(ASSISTANT_HISTORY_OPEN_KEY, 'true');
2225
const [openFloatingPanel, setOpenFloatingPanel, clearOpenFloatingPanel] =
2326
useLocalStorage<OpenFloatingPanel>(ASSISTANT_OPEN_FLOATING_PANEL_KEY);

web/packages/studio/src/routes/agents/AssistantChatRoute/AssistantTopBarChat.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import { useNavigate } from 'react-router';
2424

2525
// Static import would pull the whole chat surface into the entry chunk, since
2626
// the trigger renders in the global nav on every route.
27-
const importChatThread = () => import('@studio/routes/agents/AssistantChatRoute/AssistantChatThread');
27+
const importChatThread = () =>
28+
import('@studio/routes/agents/AssistantChatRoute/AssistantChatThread');
2829

2930
// lazy() caches a rejected import forever, so a retry needs a fresh component.
3031
const createChatThread = () =>

0 commit comments

Comments
 (0)