From f0cb664987e374b6853eafc0aa5a1281682da021 Mon Sep 17 00:00:00 2001 From: A-Mahla <> Date: Fri, 10 May 2024 17:08:01 +0200 Subject: [PATCH 01/11] updated pull_request_template.md --- docs/assets/source/.gitkeep | 0 pull_request_template.md | 71 +++++++++++++++++++++++++------------ 2 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 docs/assets/source/.gitkeep diff --git a/docs/assets/source/.gitkeep b/docs/assets/source/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/pull_request_template.md b/pull_request_template.md index e523878..39a37cb 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,33 +1,60 @@ -# Description +FILL IN THE PR DESCRIPTION HERE -Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. +FIX #xxxx (*link existing issues this PR will resolve*) -Fixes # (issue) +**BEFORE SUBMITTING, PLEASE READ THE CHECKLIST BELOW AND FILL IN THE DESCRIPTION ABOVE** -## Type of change +--- -Please delete options that are not relevant. +
+ + PR Checklist (Click to Expand) -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) -- [ ] This change requires a documentation update +
-# How Has This Been Tested? +

Thank you for your contribution to gigax! Before submitting the pull request, please ensure the PR meets the following criteria. This helps gigax maintain the code quality and improve the efficiency of the review process.

-Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration +

PR Title and Classification

+

Only specific types of PRs will be reviewed. The PR title is prefixed appropriately to indicate the type of change. Please use one of the following:

+ +

Note: If the PR spans more than one category, please include all relevant prefixes.

-- [ ] Ran unit tests on local environment -- [ ] Deployed local version of our app, and tested the feature in-game +

Code Quality

+

The PR need to meet the following code quality standards:

-# Checklist: + -- [ ] My code follows the style guidelines of this project -- [ ] I have performed a self-review of my code -- [ ] I have commented my code, particularly in hard-to-understand areas -- [ ] I have made corresponding changes to the documentation -- [ ] My changes generate no new warnings -- [ ] I have added tests that prove my fix is effective or that my feature works -- [ ] New and existing unit tests pass locally with my changes -- [ ] Any dependent changes have been merged and published in downstream modules +

Notes for Large Changes

+

Please keep the changes as concise as possible. For major architectural changes, we would expect a GitHub issue (RFC) discussing the technical design and justification. Otherwise, we will tag it with rfc-required and might not go through the PR.

+ +

What to Expect for the Reviews

+ +

The goal of the gigax team is to be a transparent reviewing machine. We would like to make the review process transparent and efficient and make sure no contributor feel confused or frustrated. However, the gigax team is small, so we need to prioritize some PRs over others. Here is what you can expect from the review process:

+ + + +

Thank You

+ +

Finally, thank you for taking the time to read these guidelines and for your interest in contributing to gigax.

+ + +
\ No newline at end of file From d886cc4da48d41f71545f6ef773b0b7bbbd15011 Mon Sep 17 00:00:00 2001 From: A-Mahla <89754740+A-Mahla@users.noreply.github.com> Date: Mon, 13 May 2024 10:12:32 +0200 Subject: [PATCH 02/11] Update pull_request_template.md Co-authored-by: tdeborde <33256624+tristandeborde@users.noreply.github.com> --- pull_request_template.md | 1 - 1 file changed, 1 deletion(-) diff --git a/pull_request_template.md b/pull_request_template.md index 39a37cb..00cb88c 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -47,7 +47,6 @@ FIX #xxxx (*link existing issues this PR will resolve*) From a047ef251414afd1995767025790320e1cc43efb Mon Sep 17 00:00:00 2001 From: A-Mahla <89754740+A-Mahla@users.noreply.github.com> Date: Mon, 13 May 2024 10:20:22 +0200 Subject: [PATCH 03/11] Delete docs/assets/source/.gitkeep --- docs/assets/source/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 docs/assets/source/.gitkeep diff --git a/docs/assets/source/.gitkeep b/docs/assets/source/.gitkeep deleted file mode 100644 index e69de29..0000000 From be660d93ee90b3c7be829901b26a7b53f836e309 Mon Sep 17 00:00:00 2001 From: A-Mahla <89754740+A-Mahla@users.noreply.github.com> Date: Mon, 13 May 2024 10:21:56 +0200 Subject: [PATCH 04/11] Update pull_request_template.md Co-authored-by: tdeborde <33256624+tristandeborde@users.noreply.github.com> --- pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pull_request_template.md b/pull_request_template.md index 00cb88c..0d66a59 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -31,7 +31,7 @@ FIX #xxxx (*link existing issues this PR will resolve*)

The PR need to meet the following code quality standards:

Notes for Large Changes

From bb6dc68489c8473e56618d5ba1bbedfeb5eba742 Mon Sep 17 00:00:00 2001 From: A-Mahla <> Date: Mon, 13 May 2024 10:55:46 +0200 Subject: [PATCH 06/11] add: ci with linter ruff --- .github/workflows/ci.yml | 37 +++++++++++++++++++++++++++++++++++++ gigax/__init__.py | 5 +++++ gigax/parse.py | 10 ++++++++++ 3 files changed, 52 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..f93bdf2 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,37 @@ +name: Pylint + +on: + pull_request: + branches: + - main + - CI/add-linter-pylint + push: + branches: + - CI/add-linter-pylint + paths: + - gigax + - gigax/**/* + - tests + - tests/**/* + # To test workflow without event trigger + workflow_dispatch: + +jobs: + build: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.9", "3.10", "3.11", "3.12"] + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v3 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install ruff + - name: Analysing the code with pylint + run: | + ruff check $(git ls-files '*.py') \ No newline at end of file diff --git a/gigax/__init__.py b/gigax/__init__.py index bd5fc92..cab73d3 100644 --- a/gigax/__init__.py +++ b/gigax/__init__.py @@ -1,4 +1,9 @@ +""" Gigax package. """ + from gigax import scene from gigax import parse from gigax import step from gigax import prompt + + +__all__ = ["scene", "parse", "step", "prompt"] diff --git a/gigax/parse.py b/gigax/parse.py index 62317ed..640d944 100644 --- a/gigax/parse.py +++ b/gigax/parse.py @@ -1,3 +1,5 @@ +"""This module contains the logic to parse a command string into a CharacterAction object.""" + import logging import re from typing import Union @@ -17,10 +19,14 @@ class ActionParsingError(Exception): + """Exception raised for errors in the action parsing.""" + pass class CharacterAction(BaseModel): + """CharacterAction class to represent a character action.""" + command: str protagonist: ProtagonistCharacter parameters: list[Union[str, int, Object]] @@ -41,6 +47,10 @@ def from_str( valid_items: list[Item], compiled_regex: re.Pattern, ) -> "CharacterAction": + """ + Parse a command string into a CharacterAction object. + """ + match = compiled_regex.match(command_str) if not match: raise ValueError("Invalid command format") From ec492b0956c8235e48d9908f863f6891482ee452 Mon Sep 17 00:00:00 2001 From: A-Mahla <> Date: Mon, 13 May 2024 10:57:58 +0200 Subject: [PATCH 07/11] change: CI naming --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f93bdf2..7f2501b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -name: Pylint +name: Linter on: pull_request: From 8d41d94938c4da91118568f33fbf13802a8b956f Mon Sep 17 00:00:00 2001 From: A-Mahla <> Date: Mon, 13 May 2024 11:00:29 +0200 Subject: [PATCH 08/11] delete: CI/add-linter-pylint branch from ci.yml --- .github/workflows/ci.yml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f2501b..5e7ae2b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,15 +4,9 @@ on: pull_request: branches: - main - - CI/add-linter-pylint push: branches: - - CI/add-linter-pylint - paths: - - gigax - - gigax/**/* - - tests - - tests/**/* + - main # To test workflow without event trigger workflow_dispatch: From cb62eb20985681b105bfbd93325da57b88702223 Mon Sep 17 00:00:00 2001 From: Tristan Date: Wed, 19 Jun 2024 21:40:34 +0200 Subject: [PATCH 09/11] Fix model name change --- tests/test_step.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_step.py b/tests/test_step.py index d3a0113..1ea81bc 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -86,9 +86,9 @@ async def test_stepper_api( ): # Get the NPC's input with pytest.raises(ValueError): - NPCStepper(model="mistral-7b-regex") + NPCStepper(model="llama_3_regex") - stepper = NPCStepper(model="mistral_7b_regex", api_key=os.getenv("API_KEY")) + stepper = NPCStepper(model="llama_3_regex", api_key=os.getenv("API_KEY")) action = await stepper.get_action( context=context, From 0fca5dd7cbb7160d84826b1ea382b9a268921d04 Mon Sep 17 00:00:00 2001 From: Tristan Date: Wed, 19 Jun 2024 21:41:42 +0200 Subject: [PATCH 10/11] Add serving capabilities --- gigax/app.py | 91 +++++++++++++++++++++++++++++++++++++++++++++++ gigax/scene.py | 16 ++++----- gigax/step.py | 9 +++-- tests/conftest.py | 1 - 4 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 gigax/app.py diff --git a/gigax/app.py b/gigax/app.py new file mode 100644 index 0000000..7262999 --- /dev/null +++ b/gigax/app.py @@ -0,0 +1,91 @@ +import logging +import os +import uvicorn +import sys + +from dotenv import load_dotenv +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse +from fastapi import FastAPI, Request, status +from fastapi.middleware.cors import CORSMiddleware +from fastapi.logger import logger as fastapi_logger +from pydantic import BaseModel + +from gigax.parse import CharacterAction, ProtagonistCharacter +from gigax.scene import ( + Character, + Item, + Location, +) +from gigax.step import NPCStepper + +load_dotenv() + +fastapi_logger.handlers = logging.getLogger("gunicorn.error").handlers +logging.basicConfig(level=logging.INFO, handlers=[logging.StreamHandler(sys.stdout)]) + +# FastAPI +app = FastAPI(root_path="/api") +origins = [ + "http://localhost", +] +app.add_middleware( + CORSMiddleware, + allow_origins=origins, + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], +) + + +logger = logging.getLogger("uvicorn") +logging.basicConfig(level=logging.INFO) + + +@app.exception_handler(RequestValidationError) +async def validation_exception_handler(request: Request, exc: RequestValidationError): + exc_str = f"{exc}".replace("\n", " ").replace(" ", " ") + logging.error(f"{request}: {exc_str}") + content = {"status_code": 10422, "message": exc_str, "data": None} + return JSONResponse( + content=content, status_code=status.HTTP_422_UNPROCESSABLE_ENTITY + ) + + +class CharacterActionRequest(BaseModel): + context: str + locations: list[Location] + NPCs: list[Character] + protagonist: ProtagonistCharacter + items: list[Item] + events: list[CharacterAction] + + +@app.post( + "/step", + response_description="Step the character", + response_model=CharacterAction, +) +async def step( + request: CharacterActionRequest, +): + # Format the prompt + stepper = NPCStepper(model="llama_3_regex", api_key=os.getenv("API_KEY")) + + return await stepper.get_action( + context=request.context, + locations=request.locations, + NPCs=request.NPCs, + protagonist=request.protagonist, + items=request.items, + events=request.events, + ) + + +@app.get("/health-check") +async def health_check(): + return {"status": "ok"} + + +if __name__ == "__main__": + uvicorn.run("__main__:app", host="0.0.0.0", port=5678, reload=True, workers=5) diff --git a/gigax/scene.py b/gigax/scene.py index 4cc8310..9538646 100644 --- a/gigax/scene.py +++ b/gigax/scene.py @@ -1,16 +1,15 @@ from enum import Enum import re -from typing import Union from pydantic import BaseModel, Field class ParameterType(str, Enum): - character = "" - location = "" - item = "" - amount = "" - content = "" - other = "" + character = "character" + location = "location" + item = "item" + amount = "amount" + content = "content" + other = "other" class Object(BaseModel): @@ -59,7 +58,7 @@ class Skill(BaseModel): name: str = Field(..., description="Skill name") description: str = Field(..., description="Skill description") - parameter_types: Union[list[ParameterType], dict] = ( + parameter_types: list[ParameterType] = ( Field( # This is a Union because Cubzh's Lua sends empty lists as empty dicts [], description="Allowed parameter types for the given skill" ) @@ -107,4 +106,3 @@ class ProtagonistCharacter(Character): memories: list[str] = Field(..., description="Memories that the character has.") quests: list[str] = Field(..., description="Quests that the character is on.") skills: list[Skill] = Field(..., description="Skills that the character can use.") - psychological_profile: str diff --git a/gigax/step.py b/gigax/step.py index 5ab3f52..216b07c 100644 --- a/gigax/step.py +++ b/gigax/step.py @@ -54,7 +54,7 @@ async def generate_api( response = await client.chat.completions.create( model=model, - messages=messages, + messages=messages, # type: ignore max_tokens=100, temperature=temperature, extra_body=dict(guided_regex=guided_regex), @@ -116,7 +116,7 @@ async def get_action( protagonist: ProtagonistCharacter, items: list[Item], events: list[CharacterAction], - ) -> CharacterAction: + ) -> CharacterAction | None: """ Prompt the NPC for an input. """ @@ -154,8 +154,7 @@ async def get_action( parsed_action = CharacterAction.from_str( res, protagonist, NPCs, locations, items, guided_regex ) + logger.info(f"NPC {protagonist.name} responded with: {parsed_action}") + return parsed_action except Exception: logger.error(f"Error while parsing the action: {traceback.format_exc()}") - - logger.info(f"NPC {protagonist.name} responded with: {parsed_action}") - return parsed_action diff --git a/tests/conftest.py b/tests/conftest.py index 4f0392a..0949816 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,7 +45,6 @@ def protagonist(current_location): parameter_types=[ParameterType.character], ) ], - psychological_profile="Determined and compassionate", ) From 653ada7d08b2baa6453516e0841e91904073d446 Mon Sep 17 00:00:00 2001 From: Tristan Date: Mon, 1 Jul 2024 17:40:55 +0200 Subject: [PATCH 11/11] Simplify return value to return strings instead of objects --- gigax/app.py | 8 ++++++-- gigax/parse.py | 25 ++++++------------------- gigax/scene.py | 2 +- gigax/step.py | 2 +- tests/test_parse.py | 2 +- 5 files changed, 15 insertions(+), 24 deletions(-) diff --git a/gigax/app.py b/gigax/app.py index 7262999..75e3b9b 100644 --- a/gigax/app.py +++ b/gigax/app.py @@ -68,11 +68,11 @@ class CharacterActionRequest(BaseModel): ) async def step( request: CharacterActionRequest, -): +) -> CharacterAction: # Format the prompt stepper = NPCStepper(model="llama_3_regex", api_key=os.getenv("API_KEY")) - return await stepper.get_action( + action = await stepper.get_action( context=request.context, locations=request.locations, NPCs=request.NPCs, @@ -80,6 +80,10 @@ async def step( items=request.items, events=request.events, ) + if action is None: + raise ValueError("No action returned from the model") + + return action @app.get("/health-check") diff --git a/gigax/parse.py b/gigax/parse.py index 640d944..4b8ccd0 100644 --- a/gigax/parse.py +++ b/gigax/parse.py @@ -8,7 +8,6 @@ from gigax.scene import ( Item, - Object, Location, Character, ProtagonistCharacter, @@ -29,7 +28,7 @@ class CharacterAction(BaseModel): command: str protagonist: ProtagonistCharacter - parameters: list[Union[str, int, Object]] + parameters: list[Union[str, int]] def __str__(self) -> str: """ @@ -42,9 +41,6 @@ def __str__(self) -> str: def from_str( command_str: str, protagonist: ProtagonistCharacter, - valid_characters: list[Character], - valid_locations: list[Location], - valid_items: list[Item], compiled_regex: re.Pattern, ) -> "CharacterAction": """ @@ -60,7 +56,8 @@ def from_str( f"Could not find a matching skill in command_str '{command_str}'" ) - command = match.lastgroup.split("_")[0] # Extract command name + # Extract the command_name while supporting multiple _ in lastgroup + command = "_".join(match.lastgroup.split("_")[:-1]) action = CharacterAction( command=command, protagonist=protagonist, parameters=[] ) @@ -75,19 +72,9 @@ def from_str( ] # Remove command prefix to get parameter type # Add parameters based on their type - if param_type == "character": - # Assuming characters are directly referred by name - action.parameters.append( - next(char for char in valid_characters if char.name == value), - ) - elif param_type == "location": - action.parameters.append( - next(loc for loc in valid_locations if loc.name == value), - ) - elif param_type == "item": - action.parameters.append( - next(item for item in valid_items if item.name == value) - ) + if param_type in ["character", "NPC", "item"]: + # Assuming these are directly referred by name + action.parameters.append(value) elif param_type == "amount": action.parameters.append(int(value)) elif param_type == "content": diff --git a/gigax/scene.py b/gigax/scene.py index 9538646..96f1c57 100644 --- a/gigax/scene.py +++ b/gigax/scene.py @@ -80,7 +80,7 @@ def to_regex( parts = [re.escape(self.name)] for param in self.parameter_types: # Each group name follows format: skillname_paramtype, without <> - group_name = f"{self.name}_{param.value[1:-1]}" + group_name = f"{self.name}_{param.value}" if param == ParameterType.character: parts.append( f"(?P<{group_name}>{'|'.join(map(re.escape, character_names))})" diff --git a/gigax/step.py b/gigax/step.py index 216b07c..d05df01 100644 --- a/gigax/step.py +++ b/gigax/step.py @@ -152,7 +152,7 @@ async def get_action( try: # Parse response parsed_action = CharacterAction.from_str( - res, protagonist, NPCs, locations, items, guided_regex + res, protagonist, guided_regex ) logger.info(f"NPC {protagonist.name} responded with: {parsed_action}") return parsed_action diff --git a/tests/test_parse.py b/tests/test_parse.py index d9dc5ae..1f717a8 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -19,7 +19,7 @@ def test_parse( # Test the from_str method character_action = CharacterAction.from_str( - test_command, protagonist, NPCs, locations, items, compiled_combined_regex + test_command, protagonist, compiled_combined_regex ) assert character_action assert character_action.command == "Attack"