feat: ui program to cb connection #26

2025-11-12T17:10:55Z

2584433 commented

2025-11-12 17:10:55 +00:00

(Migrated from git.science.uu.nl)

scheme for a program defined in the cb.
ui can now send /program messages to the cb.

To validate this merge do the following.

Make sure this branch is up to date with dev
When running the cb you can manually test a test case by inputting the comment into a new terminal. (this simulates a UI input)
Integration tests run.

ref: N25B-198

scheme for a program defined in the cb. ui can now send /program messages to the cb. To validate this merge do the following. - [x] Make sure this branch is up to date with dev - [x] When running the cb you can manually test a test case by inputting the comment into a new terminal. (this simulates a UI input) - [x] Integration tests run. ref: N25B-198

2584433 commented

2025-11-14 13:12:38 +00:00

(Migrated from git.science.uu.nl)

changed the description

2584433 commented

2025-11-14 13:35:58 +00:00

(Migrated from git.science.uu.nl)

assigned to @2584433

0950726 commented

2025-11-14 15:32:50 +00:00

(Migrated from git.science.uu.nl)

Was aan het testen met een AI agent code review (ben het i.i.g. eens met het major punt):

Thanks for the contribution!

This MR introduces a new /program endpoint for the control backend to receive program definitions from a UI. The code is well-structured, includes thorough tests, and successfully implements the core requirement.

I have a few suggestions to improve the API design and code clarity.

Major

API contract could be more direct

In src/control_backend/api/v1/endpoints/program.py, the receive_message function accepts a generic Message object and then parses a JSON string from within it.

# src/control_backend/api/v1/endpoints/program.py:16
@router.post("/program", status_code=202)
async def receive_message(program: Message, request: Request):
    """
    Receives a BehaviorProgram as a stringified JSON list inside `message`.
    Converts it into real Phase objects.
    """
    logger.debug("Received raw program: %s", program)
    raw_str = program.message  # This is the JSON string

    # Validate program
    try:
        program = Program.model_validate_json(raw_str)

This makes the API contract less clear, as the actual data structure (Program) is hidden within a string. By defining the endpoint to accept the Program model directly, FastAPI will handle the JSON deserialization and validation automatically. This simplifies the code and makes the API self-documenting.

Suggestion:

Refactor the endpoint to accept the Program model directly.

# src/control_backend/api/v1/endpoints/program.py

@router.post("/program", status_code=202)
async def receive_program(program: Program, request: Request):
    """
    Receives and validates a program, then forwards it for processing.
    """
    logger.debug("Received and validated program: %s", program.id) # Or some identifier

    # The `program` object is already a validated Pydantic model.
    # No manual validation needed.

    # send away
    topic = b"program"
    body = program.model_dump_json().encode()
    pub_socket = request.app.state.endpoints_pub_socket
    await pub_socket.send_multipart([topic, body])

    return {"status": "Program accepted"}

With this change, the client would send a JSON object matching the Program schema directly in the request body, rather than wrapping it in another JSON object. The corresponding endpoint tests would also need to be updated.

Minor

Variable naming in endpoint

In src/control_backend/api/v1/endpoints/program.py, the input parameter is named program but is typed as a Message object. The name program is then reused for the parsed Program object. This can be a bit confusing when reading the code.

Suggestion:

Use a more descriptive name for the input message, for example program_message.

# src/control_backend/api/v1/endpoints/program.py:16

async def receive_message(program_message: Message, request: Request):
    # ...
    raw_str = program_message.message
    # ...

Nitpicks

Camel case in Pydantic models

In src/control_backend/schemas/program.py, the Pydantic model fields use camelCase (e.g., nextPhaseId, phaseData). While this might be to match an external schema (e.g., from the UI), the standard Python convention is snake_case. Pydantic can automatically handle the conversion.

Suggestion:

Consider using snake_case for model fields and configure Pydantic to serialize to/from camelCase if needed by the client.

# src/control_backend/schemas/program.py

from pydantic import BaseModel, ConfigDict

class Phase(BaseModel):
    model_config = ConfigDict(alias_generator=lambda string: ''.join(word.capitalize() if i > 0 else word for i, word in enumerate(string.split('_'))))

    id: str
    name: str
    next_phase_id: str  # Use snake_case
    phase_data: "PhaseData" # Use snake_case

Testing model_validate

In test/integration/schemas/test_ui_program_message.py, the tests use Program.model_validate(program) on an object that is already a Pydantic model instance (program = base_program()). model_validate is typically used for validating raw Python dicts. This test still works, but it's not testing the primary use case of validation from a plain dictionary.

Suggestion:

To more accurately test the validation logic, create a dictionary fixture and use that for the validation test. The existing helper functions are a great starting point for this.

Was aan het testen met een AI agent code review (ben het i.i.g. eens met het major punt): Thanks for the contribution! This MR introduces a new `/program` endpoint for the control backend to receive program definitions from a UI. The code is well-structured, includes thorough tests, and successfully implements the core requirement. I have a few suggestions to improve the API design and code clarity. ### Major **API contract could be more direct** In `src/control_backend/api/v1/endpoints/program.py`, the `receive_message` function accepts a generic `Message` object and then parses a JSON string from within it. ```python # src/control_backend/api/v1/endpoints/program.py:16 @router.post("/program", status_code=202) async def receive_message(program: Message, request: Request): """ Receives a BehaviorProgram as a stringified JSON list inside `message`. Converts it into real Phase objects. """ logger.debug("Received raw program: %s", program) raw_str = program.message # This is the JSON string # Validate program try: program = Program.model_validate_json(raw_str) ``` This makes the API contract less clear, as the actual data structure (`Program`) is hidden within a string. By defining the endpoint to accept the `Program` model directly, FastAPI will handle the JSON deserialization and validation automatically. This simplifies the code and makes the API self-documenting. **Suggestion:** Refactor the endpoint to accept the `Program` model directly. ```python # src/control_backend/api/v1/endpoints/program.py @router.post("/program", status_code=202) async def receive_program(program: Program, request: Request): """ Receives and validates a program, then forwards it for processing. """ logger.debug("Received and validated program: %s", program.id) # Or some identifier # The `program` object is already a validated Pydantic model. # No manual validation needed. # send away topic = b"program" body = program.model_dump_json().encode() pub_socket = request.app.state.endpoints_pub_socket await pub_socket.send_multipart([topic, body]) return {"status": "Program accepted"} ``` With this change, the client would send a JSON object matching the `Program` schema directly in the request body, rather than wrapping it in another JSON object. The corresponding endpoint tests would also need to be updated. ### Minor **Variable naming in endpoint** In `src/control_backend/api/v1/endpoints/program.py`, the input parameter is named `program` but is typed as a `Message` object. The name `program` is then reused for the parsed `Program` object. This can be a bit confusing when reading the code. **Suggestion:** Use a more descriptive name for the input message, for example `program_message`. ```python # src/control_backend/api/v1/endpoints/program.py:16 async def receive_message(program_message: Message, request: Request): # ... raw_str = program_message.message # ... ``` ### Nitpicks **Camel case in Pydantic models** In `src/control_backend/schemas/program.py`, the Pydantic model fields use camelCase (e.g., `nextPhaseId`, `phaseData`). While this might be to match an external schema (e.g., from the UI), the standard Python convention is snake_case. Pydantic can automatically handle the conversion. **Suggestion:** Consider using `snake_case` for model fields and configure Pydantic to serialize to/from `camelCase` if needed by the client. ```python # src/control_backend/schemas/program.py from pydantic import BaseModel, ConfigDict class Phase(BaseModel): model_config = ConfigDict(alias_generator=lambda string: ''.join(word.capitalize() if i > 0 else word for i, word in enumerate(string.split('_')))) id: str name: str next_phase_id: str # Use snake_case phase_data: "PhaseData" # Use snake_case ``` **Testing `model_validate`** In `test/integration/schemas/test_ui_program_message.py`, the tests use `Program.model_validate(program)` on an object that is already a Pydantic model instance (`program = base_program()`). `model_validate` is typically used for validating raw Python dicts. This test still works, but it's not testing the primary use case of validation from a plain dictionary. **Suggestion:** To more accurately test the validation logic, create a dictionary fixture and use that for the validation test. The existing helper functions are a great starting point for this.

9828273 commented

2025-11-18 10:49:21 +00:00

(Migrated from git.science.uu.nl)

requested review from @9828273

9828273 commented

2025-11-18 10:50:34 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Make sure this branch is up to date with dev as completed

marked the checklist item **Make sure this branch is up to date with dev** as completed

9828273 commented

2025-11-18 10:50:35 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Make sure this branch is up to date with dev as incomplete

marked the checklist item **Make sure this branch is up to date with dev** as incomplete

9828273 commented

2025-11-18 10:50:57 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Integration tests run. as completed

marked the checklist item **Integration tests run.** as completed

0950726 commented

2025-11-18 11:04:40 +00:00

(Migrated from git.science.uu.nl)

If you have WSL installed, you can run this curl command to test the endpoint

curl -X POST http://localhost:8000/program -H 'content-type: application/json' -d '{
    "phases": [
        {
            "id": "phase-1",
            "name": "Generic Phase",
            "nextPhaseId": "end",
            "phaseData": {
                "norms": [
                    {
                        "id": "norm-1",
                        "name": "new norm node",
                        "value": "Pepper should talk like a fish."
                    },
                    {
                        "id": "norm-2",
                        "name": "new norm node",
                        "value": "Pepper should end sentences with \"blub\"."
                    }
                ],
                "goals": [
                    {
                        "id": "goal-1",
                        "name": "new goal node",
                        "description": "Find out what the user''s name is.",
                        "achieved": true
                    },
                    {
                        "id": "goal-2",
                        "name": "new goal node",
                        "description": "Play a yes-no guessing game where the user thinks of a word and Pepper tries to guess it by asking only questions that can be answered with \"yes\" or \"no\".",
                        "achieved": false
                    }
                ],
                "triggers": []
            }
        }
    ]
}'

If you have WSL installed, you can run this curl command to test the endpoint ``` curl -X POST http://localhost:8000/program -H 'content-type: application/json' -d '{ "phases": [ { "id": "phase-1", "name": "Generic Phase", "nextPhaseId": "end", "phaseData": { "norms": [ { "id": "norm-1", "name": "new norm node", "value": "Pepper should talk like a fish." }, { "id": "norm-2", "name": "new norm node", "value": "Pepper should end sentences with \"blub\"." } ], "goals": [ { "id": "goal-1", "name": "new goal node", "description": "Find out what the user''s name is.", "achieved": true }, { "id": "goal-2", "name": "new goal node", "description": "Play a yes-no guessing game where the user thinks of a word and Pepper tries to guess it by asking only questions that can be answered with \"yes\" or \"no\".", "achieved": false } ], "triggers": [] } } ] }' ```

2584433 commented

2025-11-18 11:33:36 +00:00

(Migrated from git.science.uu.nl)

for windows powershell inside VScode, try this:
Invoke-RestMethod -Uri "http://localhost:8000/program" -Method Post
-ContentType "application/json" `
-Body '{
"phases": [
{
"id": "phase1",
"name": "basephase",
"nextPhaseId": "phase2",
"phaseData": {
"norms": [ { "id": "n1", "name": "norm", "value": "be nice" } ],
"goals": [ { "id": "g1", "name": "goal", "description": "test", "achieved": false } ],
"triggers": [
{ "id": "t1", "label": "trigger", "type": "keyword", "value": ["stop", "exit"] }
]
}
}
]
}' | ConvertTo-Json -Depth 5

for windows powershell inside VScode, try this: Invoke-RestMethod -Uri "http://localhost:8000/program" ` -Method Post ` -ContentType "application/json" ` -Body '{ "phases": [ { "id": "phase1", "name": "basephase", "nextPhaseId": "phase2", "phaseData": { "norms": [ { "id": "n1", "name": "norm", "value": "be nice" } ], "goals": [ { "id": "g1", "name": "goal", "description": "test", "achieved": false } ], "triggers": [ { "id": "t1", "label": "trigger", "type": "keyword", "value": ["stop", "exit"] } ] } } ] }' | ConvertTo-Json -Depth 5

2584433 commented

2025-11-18 11:36:54 +00:00

(Migrated from git.science.uu.nl)

added 1 commit

39c07dd3 - refactor: made pydantic check the input.

Compare with previous version

added 1 commit <ul><li>39c07dd3 - refactor: made pydantic check the input.</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/26/diffs?diff_id=133731&start_sha=2ed2a84f130017af23882423f279d6b6486cf93d)

2584433 commented

2025-11-18 11:37:41 +00:00

(Migrated from git.science.uu.nl)

changed the description

0950726 commented

2025-11-18 12:09:35 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item When running the cb you can manually test a test case by inputting the comment into a new terminal. (this simulates a UI input) as completed

marked the checklist item **When running the cb you can manually test a test case by inputting the comment into a new terminal. (this simulates a UI input)** as completed

0950726 commented

2025-11-18 12:09:51 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Make sure this branch is up to date with dev as completed

marked the checklist item **Make sure this branch is up to date with dev** as completed

0950726 commented

2025-11-18 12:09:52 +00:00

(Migrated from git.science.uu.nl)

approved this merge request

9828273 commented

2025-11-19 09:57:27 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Make sure this branch is up to date with dev as incomplete

marked the checklist item **Make sure this branch is up to date with dev** as incomplete

2584433 commented

2025-11-19 12:17:54 +00:00

(Migrated from git.science.uu.nl)

added 21 commits

39c07dd3...df1e891a - 20 commits from branch dev
964997f8 - Merge branch 'dev' of https://git.science.uu.nl/ics/sp/2025/n25b/pepperplus-cb...

Compare with previous version

added 21 commits <ul><li>39c07dd3...df1e891a - 20 commits from branch <code>dev</code></li><li>964997f8 - Merge branch 'dev' of https://git.science.uu.nl/ics/sp/2025/n25b/pepperplus-cb...</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/26/diffs?diff_id=133802&start_sha=39c07dd3cf7d08ffb2bd69d8f5e91ac3a2674e6e)

9828273 commented

2025-11-19 12:43:06 +00:00

(Migrated from git.science.uu.nl)

marked the checklist item Make sure this branch is up to date with dev as completed

marked the checklist item **Make sure this branch is up to date with dev** as completed

9828273 commented

2025-11-19 12:43:21 +00:00

(Migrated from git.science.uu.nl)

approved this merge request

9828273 (Migrated from git.science.uu.nl) merged commit 4fc3c2a1e1 into dev

2025-11-19 14:21:01 +00:00

9828273 commented

2025-11-19 14:21:02 +00:00

(Migrated from git.science.uu.nl)

mentioned in commit 4fc3c2a1e1

mentioned in commit 4fc3c2a1e1b98ad15e3b25305e06e370553d1ee5

0950726 (Migrated from git.science.uu.nl) approved these changes 2026-02-02 13:29:06 +00:00

9828273 (Migrated from git.science.uu.nl) approved these changes 2026-02-02 13:29:06 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: pepperplus/pepperplus-cb#26