feat(tokenizer): add tokenizer loading and encoding service by nXtCyberNet · Pull Request #1265 · volcano-sh/kthena

nXtCyberNet · 2026-06-27T09:15:40Z

What type of PR is this?

What this PR does / why we need it:
implimentation of the tokenizer - based on the proposal #1225

Which issue(s) this PR fixes:
related to #1161

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

gemini-code-assist

Code Review

This pull request introduces a new FastAPI-based tokenizer service with endpoints to load, unload, list, and encode text using tokenizers downloaded from various sources (S3, PVC, HuggingFace, and ModelScope). The review feedback highlights several critical issues in the downloader module, including instantiation errors in DownloadHelper, signature mismatches and broken loop logic in downloadPVC, and a redundant method definition. Additionally, the reviewer recommends changing the encode and load endpoints from asynchronous to synchronous functions to prevent blocking the FastAPI event loop with CPU-bound and blocking I/O operations, and suggests capturing subprocess output as text for cleaner error logging.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-27T09:17:16Z

+    def __init__(self, model_uri: str, access_key: str = None, secret_key: str = None, endpoint: str = None ):
+        self.access_key = access_key
+        self.secret_key = secret_key
+        self.endpoint = endpoint


The DownloadHelper constructor requires model_uri as a positional argument, but it is instantiated without any arguments in TokenizerDownloader.__init__ (line 49). This will raise a TypeError on startup. Additionally, self.model_uri is not assigned in __init__, which will cause AttributeError when accessed by other methods. Making model_uri optional and assigning it to self.model_uri resolves these issues.

Suggested change

def __init__(self, model_uri: str, access_key: str = None, secret_key: str = None, endpoint: str = None ):

self.access_key = access_key

self.secret_key = secret_key

self.endpoint = endpoint

def __init__(self, model_uri: str = None, access_key: str = None, secret_key: str = None, endpoint: str = None ):

self.model_uri = model_uri

self.access_key = access_key

self.secret_key = secret_key

self.endpoint = endpoint

gemini-code-assist · 2026-06-27T09:17:16Z

+    def downloadPVC(self, output_dir: str ) -> str:
+        logger.info(f"Downloading tokenizer from PVC: {self.model_uri}")
+        os.makedirs(output_dir, exist_ok=True)
+
+        for file in _TOKENIZER_FILES:
+            src_file = os.path.join(self.model_uri, file)
+            dest_file = os.path.join(output_dir, file)
+            if os.path.exists(src_file):
+                shutil.copy(src_file, dest_file)
+                logger.info(f"✓ Copied {file} to {output_dir}")
+                return output_dir
+            else:
+                logger.warning(f"File {file} not found in PVC path: {self.model_uri}")
+                return None


There are multiple critical issues in downloadPVC:

It is called with two arguments (pvc_path, output_dir) in download_tokenizer_from_pvc (line 92), but its definition only accepts output_dir.

It references self.model_uri which is not set in __init__.

The loop returns prematurely on the first file iteration, meaning it will either copy only the first file and return, or return None immediately if the first file is missing, without checking the rest of the files.

Updating the signature and fixing the loop logic ensures all tokenizer files are copied correctly.

def downloadPVC(self, pvc_path: str, output_dir: str) -> str: logger.info(f"Downloading tokenizer from PVC: {pvc_path}") os.makedirs(output_dir, exist_ok=True) copied_any = False for file in _TOKENIZER_FILES: src_file = os.path.join(pvc_path, file) dest_file = os.path.join(output_dir, file) if os.path.exists(src_file): shutil.copy(src_file, dest_file) logger.info(f"✓ Copied {file} to {output_dir}") copied_any = True else: logger.warning(f"File {file} not found in PVC path: {pvc_path}") if copied_any: return output_dir raise FileNotFoundError(f"No tokenizer files found in PVC path: {pvc_path}")

gemini-code-assist · 2026-06-27T09:17:16Z

+
+
+@router.post("/v1/encode")
+async def encode(req: EncodeRequest):


The encode route handler is defined as async def, but tokenization is a CPU-bound operation. Running CPU-bound tasks in an async def function blocks the FastAPI event loop, preventing other requests from being processed concurrently. Defining it as a standard synchronous def function allows FastAPI to run it in an external thread pool, preserving event loop concurrency.

Suggested change

async def encode(req: EncodeRequest):

def encode(req: EncodeRequest):

gemini-code-assist · 2026-06-27T09:17:16Z

+
+
+@router.post("/v1/load")
+async def load(req: LoadRequest):


The load route handler is defined as async def, but downloading tokenizers from S3, PVC, HuggingFace, or ModelScope involves blocking network and disk I/O. Running blocking I/O in an async def function blocks the FastAPI event loop. Defining it as a standard synchronous def function allows FastAPI to run it in an external thread pool, preventing event loop blocking.

Suggested change

async def load(req: LoadRequest):

def load(req: LoadRequest):

gemini-code-assist · 2026-06-27T09:17:16Z

+    def modelscope_download(modelroute: str, revision: str | None = None ) -> Tokenizer:
+        snapshot_path = snapshot_download(
+            repo_id=modelroute,
+            revision=revision,
+            allow_patterns=_TOKENIZER_FILES,
+            )
+
+        tokenizer = Tokenizer.from_pretrained(snapshot_path)
+        return tokenizer


The modelscope_download method is defined inside DownloadHelper but lacks self or @staticmethod, making it invalid as an instance method. Furthermore, it is completely redundant and unused because TokenizerDownloader has its own download_tokenizer_from_modelscope method. It should be removed to keep the codebase clean.

gemini-code-assist · 2026-06-27T09:17:16Z

+            cmd.extend(['--include', file])
+
+        try:
+            subprocess.run(cmd, env=self._prepare_environment(), check=True, capture_output=True)


When calling subprocess.run with capture_output=True, the output is captured as bytes by default. Setting text=True ensures that e.stderr is captured as a string, which makes the error log much cleaner and avoids printing byte strings (e.g., b'...').

Suggested change

subprocess.run(cmd, env=self._prepare_environment(), check=True, capture_output=True)

subprocess.run(cmd, env=self._prepare_environment(), check=True, capture_output=True, text=True)

LiZhenCheng9527

Please add license

LiZhenCheng9527 · 2026-06-30T02:11:38Z

+    def __init__(self, model_uri: str, access_key: str = None, secret_key: str = None, endpoint: str = None ):
+        self.access_key = access_key
+        self.secret_key = secret_key
+        self.endpoint = endpoint


nXtCyberNet · 2026-06-30T03:20:31Z

@LiZhenCheng9527 hi , thanks for review , i have a small request could you please review the pr #1171 , its the current blocker for this . Thanks in advance!

Copilot

Pull request overview

Adds a new Python FastAPI “tokenizer service” under python/kthena/tokenizer/ intended to load tokenizers from multiple backends and provide an /v1/encode API, aligning with the proposal to move away from heuristic token counting toward model-accurate tokenization.

Changes:

Introduces a FastAPI app entrypoint and router exposing /v1/load, /v1/unload, /v1/list, and /v1/encode.
Adds Pydantic request schemas for encode/load/unload.
Implements tokenizer download/loading utilities supporting hf:// (default), ms://, s3://, and pvc:// URIs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
python/kthena/tokenizer/main.py	Creates the FastAPI app and includes the tokenizer router.
python/kthena/tokenizer/app/schema.py	Defines request models for tokenizer load/unload/encode endpoints.
python/kthena/tokenizer/app/routes.py	Implements the HTTP endpoints and encoding logic.
python/kthena/tokenizer/app/downloader.py	Adds in-memory tokenizer registry plus download helpers for multiple URI schemes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+class DownloadHelper:
+    def __init__(self, model_uri: str, access_key: str = None, secret_key: str = None, endpoint: str = None ):
+        self.access_key = access_key
+        self.secret_key = secret_key
+        self.endpoint = endpoint
+


+    def downloadPVC(self, output_dir: str ) -> str:
+        logger.info(f"Downloading tokenizer from PVC: {self.model_uri}")
+        os.makedirs(output_dir, exist_ok=True)
+
+        for file in _TOKENIZER_FILES:
+            src_file = os.path.join(self.model_uri, file)
+            dest_file = os.path.join(output_dir, file)
+            if os.path.exists(src_file):
+                shutil.copy(src_file, dest_file)
+                logger.info(f"✓ Copied {file} to {output_dir}")
+                return output_dir
+            else:
+                logger.warning(f"File {file} not found in PVC path: {self.model_uri}")
+                return None


+    def get_tokenizer(self, model_server_id: str) -> Tokenizer:
+        return self.tokenizers.get(model_server_id)


+import asyncio
+from fastapi import APIRouter, HTTPException
+from .schema import EncodeRequest, LoadRequest, UnLoadRequest
+from .downloader import TokenizerManager, TokenizerDownloader
+import logging
+
+router = APIRouter()
+
+logger = logging.getLogger(__name__)
+
+manager = TokenizerManager()
+downloader = TokenizerDownloader()
+
+
+@router.post("/v1/encode")
+async def encode(req: EncodeRequest):
+    encoded, num_tokens = encoder(req.model_server_id,req.text )


+
+@router.post("/v1/load")
+async def load(req: LoadRequest):
+    loop = asyncio.get_event_loop()


+@router.post("/v1/load")
+async def load(req: LoadRequest):
+    loop = asyncio.get_event_loop()
+
+    try:
+        tokenizer = await loop.run_in_executor(None,downloader.download_tokenizer,req.model_uri,req.modelrevision)
+        manager.load_tokenizer(req.model_server_id,tokenizer)


+class LoadRequest(BaseModel):
+    model_server_id: str
+    model_uri: str
+    modelrevision: str | None = None
+
+
+class UnLoadRequest(BaseModel):
+    model_server_id: str


Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 13 comments.

+class LoadRequest(BaseModel):
+    model_server_id: str
+    model_uri: str
+    modelrevision: str | None = None


+class UnLoadRequest(BaseModel):
+    model_server_id: str


+    loop = asyncio.get_event_loop()
+
+    try:
+        tokenizer = await loop.run_in_executor(None,downloader.download_tokenizer,req.model_uri,req.modelrevision)
+        manager.load_tokenizer(req.model_server_id,tokenizer)


+@router.post("/v1/load")
+async def load(req: LoadRequest):
+    loop = asyncio.get_event_loop()
+
+    try:
+        tokenizer = await loop.run_in_executor(None,downloader.download_tokenizer,req.model_uri,req.modelrevision)
+        manager.load_tokenizer(req.model_server_id,tokenizer)
+
+    except Exception as e:
+        logger.exception(f"Failed to load tokenizer for {req.model_server_id}")
+
+        raise HTTPException(status_code=500,detail=f"Failed to load tokenizer: {e}")
+
+    return {
+        "message": "Tokenizer loaded successfully",
+        "model_server_id": req.model_server_id,
+        "loaded": True
+    }
+
+
+@router.post("/v1/unload")
+async def unload(req: UnLoadRequest):
+    manager.unload_tokenizer(req.model_server_id)
+
+    return {
+        "message": "Tokenizer unloaded successfully",
+        "model_server_id": req.model_server_id
+    }
+
+
+@router.get("/v1/list")
+async def list_tokenizers():
+    return {
+        "message": "List of loaded tokenizers",
+        "tokenizers": manager.list_tokenizers()
+    }


+    def get_tokenizer(self, model_server_id: str) -> Tokenizer:
+        return self.tokenizers.get(model_server_id)


+from fastapi import FastAPI
+from app.routes import router as routes


@@ -0,0 +1,17 @@
+from pydantic import BaseModel


@@ -0,0 +1,85 @@
+import asyncio


+
+import shutil


+    def modelscope_download(modelroute: str, revision: str | None = None ) -> Tokenizer:
+        snapshot_path = snapshot_download(
+            repo_id=modelroute,
+            revision=revision,
+            allow_patterns=_TOKENIZER_FILES,
+            )
+
+        tokenizer = Tokenizer.from_pretrained(snapshot_path)
+        return tokenizer


Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

volcano-sh-bot · 2026-07-08T19:09:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign git-malu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

pkg/OWNERS
python/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

volcano-sh-bot requested review from git-malu and hzxuzhonghu June 27, 2026 09:15

volcano-sh-bot added the size/L label Jun 27, 2026

gemini-code-assist Bot reviewed Jun 27, 2026

View reviewed changes

nXtCyberNet marked this pull request as draft June 27, 2026 09:19

volcano-sh-bot added the do-not-merge/work-in-progress label Jun 27, 2026

nXtCyberNet changed the title ~~Feat/tokenizer~~ feat(tokenizer): add tokenizer loading and encoding service Jun 27, 2026

nXtCyberNet mentioned this pull request Jun 27, 2026

bug: Output token rate limiting can be bypassed and may need a reservation-based design #1266

Open

LiZhenCheng9527 reviewed Jun 30, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 7, 2026 08:30

hzxuzhonghu force-pushed the feat/tokenizer branch from 50f362c to 07d1ad8 Compare July 7, 2026 08:30

Copilot started reviewing on behalf of hzxuzhonghu July 7, 2026 08:30 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

nXtCyberNet added 4 commits July 8, 2026 19:38

added tokenizer endpoints and managing logic

4b8af5a

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

completed the full workflow for hf

560ad82

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

added modulerouteparsing

42f7db4

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

solve the load endpoint i/o blocking

8df3bdb

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot AI review requested due to automatic review settings July 8, 2026 14:08

nXtCyberNet force-pushed the feat/tokenizer branch from 07d1ad8 to 8df3bdb Compare July 8, 2026 14:08

Copilot started reviewing on behalf of nXtCyberNet July 8, 2026 14:08 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

added tokenizer helpers

55bdd7a

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot AI review requested due to automatic review settings July 8, 2026 19:09

Copilot AI reviewed Jul 8, 2026

.

fb2a6ba

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot AI review requested due to automatic review settings July 8, 2026 20:35

Copilot AI reviewed Jul 8, 2026



		@router.post("/v1/encode")
		async def encode(req: EncodeRequest):

	async def encode(req: EncodeRequest):
	def encode(req: EncodeRequest):

	async def load(req: LoadRequest):
	def load(req: LoadRequest):

	subprocess.run(cmd, env=self._prepare_environment(), check=True, capture_output=True)
	subprocess.run(cmd, env=self._prepare_environment(), check=True, capture_output=True, text=True)

		def get_tokenizer(self, model_server_id: str) -> Tokenizer:
		return self.tokenizers.get(model_server_id)

		from fastapi import FastAPI
		from app.routes import router as routes

Uh oh!

Conversation

nXtCyberNet commented Jun 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

LiZhenCheng9527 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

LiZhenCheng9527 left a comment

Choose a reason for hiding this comment

Uh oh!

LiZhenCheng9527 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

nXtCyberNet commented Jun 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

volcano-sh-bot commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants