New huge version

[Timik232]

A completely redesigned framework, many new features, improved quality of life, improved quality and reduced memory usage, fixed bugs, and more

Summary by CodeRabbit

• New Features

  • Added advanced DPO and GRPO training capabilities (including configurable rewards).
  • Introduced RKLLM conversion tooling for Rockchip NPU deployment.
  • Expanded evaluation/testing with a DeepEval-backed framework and richer metrics.
  • Added Optuna hyperparameter optimization and a Fire-based CLI workflow.

• Chores

  • Updated Docker images/compose paths and improved pipeline orchestration (including RKLLM/Ollama packaging).
  • Refreshed configuration structure with improved validation/logging support.

• Documentation

  • Major documentation expansion (Russian README, API reference, training/deployment guides).

[coderabbitai]

Warning

Review limit reached

@Timik232, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 42 minutes and 34 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81ebf722-a576-43c6-a5ee-389e122c55ca

📥 Commits

Reviewing files that changed from the base of the PR and between b3cdc73 and 9cc6037.

📒 Files selected for processing (6)

• .dvc/config
• data.dvc
• pyproject.toml
• training_model/one_file_train.py
• training_model/private_api.py
• training_model/utils.py

📝 Walkthrough

Walkthrough

Comprehensive refactor adding DPO/GRPO training modes, DeepEval evaluation framework, Docker/RKLLM deployment tooling, centralized configuration management with validation, extensive documentation, and development utilities. Removes legacy testing modules and datasets while reorganizing evaluation/testing around the new evaluation package.

Changes

Core Training, Evaluation & Configuration

Layer / File(s)	Summary
Configuration schema and multi-method training setup conf/config.yaml	Restructures config with model (model_name, quant, rkllm, lora, model_type), generation (sampling params), training (batches, epochs, precision, adam_mini), memory_management, data_preparation, grpo (reward function, vllm settings), dpo (hyperparameters), optuna (hyperparameter tuning), and expanded testing/logging sections with DeepEval and MLflow integration.
Training pipeline refactor and utilities training_model/__init__.py, training_model/__main__.py, training_model/config_validation.py, training_model/dpo_train.py, training_model/grpo_train.py, training_model/data_preparation.py, training_model/exceptions.py, training_model/logging_config.py, training_model/logging_utils.py, training_model/memory_utils.py	Adds LLMLoRaCLI for Fire-based CLI, comprehensive config validation (model/training/paths/DPO/GRPO/logging/environment), DPO and GRPO training modules with data prep and reward functions, custom exception hierarchy, colored logging with progress tracking, MLflow backend integration with artifact/config logging, and memory utilities with cleanup hooks.
Evaluation framework and model wrappers evaluation/__init__.py, evaluation/deepeval_integration.py, evaluation/model_evaluation.py, evaluation/game_evaluation.py, testing_model/models.py, testing_model/__init__.py, testing_model/__main__.py	Migrates DeepEval testing to evaluation/ with lazy-loaded models (Mistral, local, custom API), sync/async evaluation tests, and dataset-driven orchestration; refactors testing_model to provide model wrapper classes and integrate DeepEval metrics into test execution.

Infrastructure & Deployment

Layer / File(s)	Summary
Docker and RKLLM conversion service Dockerfile, Dockerfile_cuda, Dockerfile_ollama, docker-compose.yaml, rkllm_files/Dockerfile.rkllm, rkllm_files/entrypoint.sh, rkllm_files/rkllm_converting.py	Reorganizes Docker workspace from /training_model to /app, adds Docker CLI installation, consolidates llama.cpp CUDA build earlier in the flow, updates Ollama paths, introduces RKLLM converter service with dedicated Dockerfile and Python conversion script, and declares MLflow network in docker-compose.
Pre-commit, dependencies, and DVC .pre-commit-config.yaml, pyproject.toml, requirements.txt, .flake8, mypy.ini, .dvc/config, .dvc/.gitignore, .dvcignore, .gitignore	Bumps/adds pre-commit hooks (bandit, codespell, split ruff/black, remove flake8), migrates pyproject.toml to poetry format with CUDA cu124 support and new deps (wandb, mlflow, fire, optuna), refreshes requirements.txt with cu124 variants, removes mypy/flake8 options, adds DVC remote/cache/ignore config, and expands repository gitignore patterns.
Pipeline and entry point updates main.py, run_pipeline.sh	Replaces legacy test entry point with Fire CLI call to LLMLoRaCLI, switches training/testing to poetry-based invocations, adds conditional RKLLM conversion flow with docker execution, and improves Ollama readiness/blob upload handling with HTTP status awareness.

Testing, Tools & Documentation

Layer / File(s)	Summary
VLLM test refactor and dev utilities testing_model/vllm_test.py, dev.py, tools/dataset/*	Extracts SYSTEM_PROMPT constant in vllm_test.py, updates dataset UTF-8 encoding handling; adds dev.py CLI for setup/format/lint/test/docs/env/validate; introduces dataset analysis scripts (coverage, quality, problem finders, validators).
Test infrastructure and suites tests/__init__.py, tests/README.md, tests/test_requirements.txt, tests/integration/test_training_functions.py, tests/unit/test_dpo_grpo.py	Adds test package structure, integration tests for training functions (fixtures, utility tests), comprehensive unit tests for DPO/GRPO (config validation, data prep, reward functions, integration), and test requirements file.
Documentation overhaul README.md, docs/conf.py, docs/*.rst, docs/*.md, .readthedocs.yaml	Completely rewrites README with quick start and feature overview; adds comprehensive Sphinx docs (API reference, training methods, Hydra config, RKLLM conversion, Docker deployment, evaluation, contributing guide, fire CLI); updates Sphinx configuration with proper metadata, extensions, intersphinx, HTML/PDF output settings, and napoleon docstring support.
Removed legacy code and datasets testing_model/deepeval_func.py (deleted), testing_model/test.py (deleted), data/test_ru.json, data/test_eng.json, data/daedalus_eng.json, data/values.txt, legacy/llama31_train.py	Removes duplicate testing harnesses, example datasets, and legacy monolithic training script in favor of modular, config-driven pipeline.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as main.py<br/>(Fire CLI)
    participant LLMLoRaCLI
    participant ConfigVal as config_validation
    participant Hydra
    participant TrainMain as training_model/__main__
    participant DPOGRPOTrain as dpo_train/grpo_train
    participant Eval as evaluation
    participant MLflow

    User->>CLI: python main.py train --config_path conf model=llama-7b
    CLI->>LLMLoRaCLI: Fire(LLMLoRaCLI).train()
    LLMLoRaCLI->>Hydra: initialize_config_dir + compose(overrides)
    Hydra-->>LLMLoRaCLI: cfg (DictConfig)
    LLMLoRaCLI->>ConfigVal: validate_complete_config(cfg)
    ConfigVal->>ConfigVal: check model/training/paths/dpo/grpo/logging
    ConfigVal-->>LLMLoRaCLI: bool (pass/fail + summary)
    LLMLoRaCLI->>TrainMain: main_train(data_dir, cfg)
    alt cfg.optuna.enabled
        TrainMain->>TrainMain: optuna_optimize(data_dir, cfg)
    else standard training
        TrainMain->>TrainMain: resolve data_dir, ensure paths exist
        TrainMain->>DPOGRPOTrain: dpo_train or grpo_train
        DPOGRPOTrain->>DPOGRPOTrain: validate config, prepare datasets
        DPOGRPOTrain->>MLflow: log_training_config, log_dataset_samples
        DPOGRPOTrain->>DPOGRPOTrain: configure trainer with PEFT/optimizer
        DPOGRPOTrain->>DPOGRPOTrain: train with memory cleanup hooks
        DPOGRPOTrain->>MLflow: log_training_artifacts, log evaluation metrics
    end
    alt cfg.testing.enabled
        TrainMain->>Eval: test_llm or run_all_deepeval_metrics
        Eval->>Eval: load dataset, instantiate evaluation models
        Eval->>Eval: run async/sync evaluation tests
        Eval-->>Eval: aggregate metrics per function
    end
    TrainMain-->>LLMLoRaCLI: results
    LLMLoRaCLI-->>User: exit code

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Rationale:

• Heterogeneous scope: Cross-cutting changes across training algorithms (DPO/GRPO), evaluation framework (DeepEval integration), Docker/deployment (RKLLM, compose network), configuration management (validation, Hydra), and extensive documentation.
• Dense new logic:
  • training_model/dpo_train.py and training_model/grpo_train.py: data transformation, reward function correctness, trainer wiring, PEFT configuration, memory cleanup orchestration.
  • evaluation/deepeval_integration.py and evaluation/model_evaluation.py: lazy imports, async/sync paths, model selection fallbacks, dataset-driven evaluation, thread-safety.
  • rkllm_files/rkllm_converting.py: model format detection, RKLLM initialization, quantization/optimization options, error handling.
  • training_model/logging_utils.py and training_model/config_validation.py: MLflow integration, config assumption validation, side effects on file system and environment.
• Areas requiring extra scrutiny:
  • Reward function implementations (JSON validation, action extraction, combined weighting) and correctness against test data.
  • Async evaluation paths (semaphores, rate limiting, error handling) and thread-safety in parallel test execution.
  • Path resolution and directory creation in training_model/__main__.py with Hydra working directory context.
  • Docker path changes (/app symlinks, llama.cpp build dependencies, socket mounts) and docker-compose network configuration.
  • Removal of legacy testing_model/deepeval_func.py and testing_model/test.py; ensure no dangling imports in init.py and test harnesses.
• File spread: ~70 files modified, several new modules, multiple removals, intertwined dependencies.

Possibly related PRs

• Timik232/LLM_LoRa#8 — overlaps on pre-commit config, flake8 removal, ruff/black upgrades, and Sphinx docs updates; direct code-level conflict risk.
• Timik232/LLM_LoRa#3 — related to Docker/compose, run_pipeline.sh Ollama integration, and test refactoring.

🐰 I hopped through configs, code, and flows,
Where DPO and GRPO dance in rows.
Docker sings beneath the moon so bright,
With /app and rewards shining in the light.
Tests and tools now play their part — hooray! 🎉

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch develop

[Copilot]

Pull Request Overview

This is a comprehensive framework redesign introducing multiple training methods (SFT, DPO, GRPO), improved architecture, memory optimization utilities, and enhanced evaluation capabilities with Fire CLI integration.

Key Changes

• Added DPO and GRPO training implementations with multiple reward functions
• Introduced Fire CLI for modern command-line interface
• Implemented memory management utilities and MLflow logging integration
• Created comprehensive test suite and custom exception hierarchy

Reviewed Changes

Copilot reviewed 73 out of 90 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
training_model/memory_utils.py	New memory management utilities with CUDA error handling
training_model/logging_utils.py	MLflow integration and dataset logging utilities
training_model/logging_config.py	Enhanced logging with named loggers and structured output
training_model/grpo_train.py	GRPO training implementation with reward functions
training_model/exceptions.py	Custom exception hierarchy for better error handling
training_model/dpo_train.py	DPO training implementation
training_model/data_preparation.py	Data processing utilities for different formats
training_model/config_validation.py	Comprehensive configuration validation framework
training_model/main.py	Updated entry point with Optuna integration
training_model/init.py	Fire CLI interface and expanded exports
tests/	Comprehensive unit and integration test suite
pyproject.toml	Updated dependencies and tooling configuration
main.py	Fire CLI integration as primary entry point

---

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

[coderabbitai]

Actionable comments posted: 36

🧹 Nitpick comments (23)

training_model/exceptions.py (1)

17-95: Excellent exception coverage with clear documentation.

The specialized exception classes cover all major failure domains in the framework, and the docstrings provide clear guidance on when each exception should be raised. The hierarchy enables both specific error catching (ConfigurationError) and general framework error handling (LLMLoRaError).

Optional enhancement for future consideration:

If you find yourself needing to capture additional error context (error codes, original exceptions, configuration values, etc.), consider adding custom __init__ methods to store context as attributes:

class ConfigurationError(LLMLoRaError):
    """Raised when configuration validation or loading fails."""
    
    def __init__(self, message, config_key=None, config_value=None):
        super().__init__(message)
        self.config_key = config_key
        self.config_value = config_value

This can improve debugging and error reporting, but it's not necessary at this stage.

docs/requirements.txt (1)

1-4: Consider pinning package versions for reproducible documentation builds.

None of the documentation dependencies specify version constraints, which could lead to non-reproducible builds or breakage if packages introduce breaking changes.

Consider applying version pinning:

-sphinx
-sphinx_rtd_theme
-autodocsumm
-myst-parser
+sphinx>=7.0,<8.0
+sphinx_rtd_theme>=2.0,<3.0
+autodocsumm>=0.2,<0.3
+myst-parser>=2.0,<3.0

.dvcignore (1)

1-3: Consider adding ignore patterns to improve DVC performance.

The file currently contains only documentation links. Adding common patterns (e.g., *.pyc, __pycache__, .git/) could improve DVC performance as suggested in the comments.

Example patterns to consider:

# Python
__pycache__/
*.py[cod]
*.egg-info/

# IDE
.vscode/
.idea/

# OS
.DS_Store
Thumbs.db

.gitignore (1)

185-185: Remove duplicate data directory ignore pattern.

The pattern data/ at line 185 and /data at line 204 both ignore data directories. The first pattern data/ will match data directories at any level, making the second pattern redundant.

Apply this diff to remove the duplicate:

-/data

Also applies to: 204-204

tools/dataset/find_threat_question_problems.py (2)

33-33: Remove unused question_indicators variable.

The question_indicators list is defined but never used in the script.

Apply this diff:

-# Question patterns in user input
-question_indicators = ["?", "ты", "можешь", "будешь", "собираешься"]
-

---

60-60: Move import to top of file.

PEP 8 requires imports at the top of the file. The defaultdict import should be moved to line 6.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path

And remove the import from line 60:

 # Group by type
-from collections import defaultdict
-
 by_type = defaultdict(list)

tools/dataset/find_question_action_problems.py (1)

39-39: Move import to top of file.

PEP 8 requires imports at the top of the file.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path

And remove the import from line 39:

-from collections import defaultdict
-
 by_action = defaultdict(list)

tools/dataset/find_future_tense_problems.py (1)

52-52: Move import to top of file.

PEP 8 requires imports at the top of the file.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path

And remove from line 52:

-from collections import defaultdict
-
 by_action = defaultdict(list)

tools/dataset/analyze_dataset.py (1)

59-68: Rename unused loop variable to indicate it's intentionally unused.

The key variable in the loop is not used within the body.

Apply this diff:

-for key, example in examples.items():
+for _key, example in examples.items():
     prompt_data = example.get("prompt", {})

tools/dataset/validate_extended_dataset.py (1)

1-136: Consider adding CLI argument support for flexibility.

The script would be more flexible with command-line arguments to specify the dataset path and validation thresholds.

Consider adding argparse support:

import argparse

def main():
    parser = argparse.ArgumentParser(description="Validate extended dataset")
    parser.add_argument(
        "--dataset",
        type=Path,
        default=Path(__file__).parent.parent.parent / "data" / "dataset_ru_extended.json",
        help="Path to dataset file"
    )
    args = parser.parse_args()
    
    data_path = args.dataset
    # ... rest of validation logic

if __name__ == "__main__":
    main()

training_model/data_preparation.py (1)

48-52: Consider moving imports to module level or using a strategy pattern.

The conditional import reduces code clarity and can make dependencies harder to track.

Consider one of these approaches:

Option 1: Import at module level with alias

from typing import Any
from evaluation.model_evaluation import get_user_prompt as get_game_user_prompt

def dataset_to_json(
    dataset: dict[str, Any],
    filename: str | Path,
    method: str = "classic",
) -> list[dict[str, str]]:
    # ... existing code ...
    
    if method == "game":
        user_message = get_game_user_prompt(example.get("prompt", {}))
    else:
        user_message = get_classic_user_prompt(instruction)

Option 2: Strategy pattern for better extensibility

def _get_classic_messages(example):
    instruction = str(example.get("instruction", ""))
    user_message = get_classic_user_prompt(instruction)
    bot_message = str(example.get("output", ""))
    return user_message, bot_message

def _get_game_messages(example):
    from evaluation.model_evaluation import get_user_prompt as get_game_user_prompt
    user_message = get_game_user_prompt(example.get("prompt", {}))
    bot_message = str(example.get("answer", ""))
    return user_message, bot_message

METHODS = {
    "classic": _get_classic_messages,
    "game": _get_game_messages,
}

dev.py (1)

38-41: Consider more specific exception handling.

While broad exception handling in a dev tool is acceptable for robustness, more specific exceptions would provide better error messages.

Consider catching specific exceptions:

 try:
     import torch
-except Exception:
+except (ImportError, ModuleNotFoundError):
     torch = None

Similar changes could be applied to the exception handlers at lines 377, 390, and 396 in the show_env_info() function.

evaluation/model_evaluation.py (6)

31-61: Consider refactoring to eliminate circular import.

The comment indicates this function was moved here to avoid a circular import. While the current solution works, circular dependencies often point to architectural issues that could be resolved through better module organization.

Consider extracting shared prompt utilities to a dedicated module (e.g., evaluation.prompt_utils or common.prompts) that both evaluation and training_model can import without creating cycles.

---

101-119: Simplify file writing logic.

The function opens the file twice (once to clear it, once to append). This can be simplified to a single write operation.

-    with Path(filename).open("w", encoding="utf-8") as file:
-        file.write("")
-
     for row in dataset:
         system_message = system
         user_message = get_user_prompt(dataset[row]["prompt"])
-        # user_message = str(dataset[row]['prompt'])
         bot_message = str(dataset[row]["answer"])
 
         json_object = {
             "system": system_message,
             "user": user_message,
             "bot": bot_message,
         }
 
         json_objects.append(json_object)
 
     with Path(filename).open("w", encoding="utf-8") as file:
-        file.write(json.dumps(json_objects, indent=4, ensure_ascii=False))
+        json.dump(json_objects, file, indent=4, ensure_ascii=False)

---

557-559: Remove redundant exception object from logging call.

When using logger.exception(), the exception is automatically included in the log. Passing it explicitly is redundant.

         except Exception as e:
-            logger.exception("Error processing test case %d: %s", number, e)
+            logger.exception("Error processing test case %d", number)

---

610-619: Clean up code quality issues.

Two minor issues:

1. Line 612: Unused noqa directive for SIM113 (not enabled in your linter config)
2. Line 618: Redundant exception object in logger.exception call

         for future in as_completed(futures):
-            completed += 1  # noqa: SIM113
+            completed += 1
             if completed % 10 == 0 or completed == total_tests:
                 logger.info(f"Progress: {completed}/{total_tests} tests completed")
             try:
                 future.result()
             except Exception as e:
-                logger.exception(f"Unexpected error in parallel test execution: {e}")
+                logger.exception("Unexpected error in parallel test execution")

---

825-830: Remove unused noqa directive.

Line 828 has an unused noqa directive for SIM113, which is not enabled in your linter configuration.

         await coro
-        completed += 1  # noqa: SIM113
+        completed += 1
         if completed % 10 == 0 or completed == total_tests:

---

962-964: Use logging.exception for exception logging.

Lines 963-964 use logger.error() followed by logger.exception(). Since logger.exception() automatically includes the error message and traceback, the logger.error() call is redundant.

         except Exception as e:
-            logger.error(f"✗ Test function {test.__name__} failed with error:")
-            logger.exception(e)
+            logger.exception(f"✗ Test function {test.__name__} failed with error")

docs/CONTRIBUTING.md (1)

57-61: Consider matching GitHub username case for consistency

The clone URL uses timik232 (lowercase), while the PR author is Timik232 (capital T). Although GitHub URLs are case-insensitive, matching the actual username casing improves consistency.

    # Fork on GitHub, then clone your fork
-   git clone https://github.com/timik232/LLM_LoRa.git
+   git clone https://github.com/Timik232/LLM_LoRa.git
    cd LLM_LoRa

tests/integration/test_training_functions.py (1)

58-60: Remove unused tokenize parameter or prefix with underscore.

The tokenize parameter is not used in the apply_chat_template mock function. Either remove it or rename it to _tokenize to indicate it's intentionally unused.

Apply this diff:

-    def apply_chat_template(messages: list, tokenize: bool = False) -> str:
+    def apply_chat_template(messages: list, _tokenize: bool = False) -> str:
         parts = [f"{m['role'].upper()}: {m['content']}" for m in messages]
         return "\n".join(parts)

Based on static analysis hints

training_model/__init__.py (3)

5-5: Remove unnecessary noqa directive.

The # noqa: F401 comment is unnecessary if the F401 rule is not enabled in your linter configuration.

-from typing import Optional  # noqa: F401
+from typing import Optional

Or remove the import entirely if it's truly unused.

Based on static analysis hints

---

321-341: Prefix unused parameters with underscore in wrapper functions.

Similar to the issue in testing_model/__main__.py, these wrapper functions have unused parameters (correct_answer) that should be prefixed with underscore to indicate they're intentionally unused for signature compatibility.

Example for wrapper_unintended_answer_mention:

             def wrapper_unintended_answer_mention(
-                model_answer: str, correct_answer: str, **kwargs: dict
+                model_answer: str, _correct_answer: str, **kwargs: dict
             ) -> bool:
                 user_input = kwargs.get("user_input", "")
                 return test_unintended_answer_mention(
-                    self.cfg, user_input, model_answer, correct_answer
+                    self.cfg, user_input, model_answer, _correct_answer
                 )

Apply similar changes to the other wrapper functions on lines 329-332 and 334-341.

Based on static analysis hints

---

116-116: Consider extracting long error messages to constants.

The long exception messages on lines 116, 160, and 405 could be extracted as module-level constants or moved to custom exception classes for better maintainability. This is a minor style improvement and not critical.

Example:

# At module level
CONFIG_VALIDATION_ERROR = "Configuration validation failed"

# In method
if not validate_complete_config(self.cfg):
    logger.error(f"{CONFIG_VALIDATION_ERROR} - aborting training")
    raise ValueError(CONFIG_VALIDATION_ERROR)

Based on static analysis hints

Also applies to: 160-160, 405-405

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df708b and c348a98.

⛔ Files ignored due to path filters (2)

• data/translate.csv is excluded by !**/*.csv
• poetry.lock is excluded by !**/*.lock

📒 Files selected for processing (73)

• .dvc/.gitignore (1 hunks)
• .dvc/config (1 hunks)
• .dvcignore (1 hunks)
• .flake8 (0 hunks)
• .gitignore (1 hunks)
• .pre-commit-config.yaml (1 hunks)
• Dockerfile (3 hunks)
• Dockerfile_cuda (1 hunks)
• Dockerfile_ollama (1 hunks)
• README.md (1 hunks)
• conf/config.yaml (1 hunks)
• data.dvc (1 hunks)
• data/daedalus_eng.json (0 hunks)
• data/test_eng.json (0 hunks)
• data/test_ru.json (0 hunks)
• data/values.txt (0 hunks)
• dev.py (1 hunks)
• docker-compose.yaml (1 hunks)
• docs/CONTRIBUTING.md (1 hunks)
• docs/README_RU.md (1 hunks)
• docs/api_reference.rst (1 hunks)
• docs/conf.py (2 hunks)
• docs/docker_deployment.rst (1 hunks)
• docs/evaluation.rst (1 hunks)
• docs/fire_cli.rst (1 hunks)
• docs/hydra_config.rst (7 hunks)
• docs/index.rst (1 hunks)
• docs/requirements.txt (1 hunks)
• docs/rkllm_conversion.rst (1 hunks)
• docs/testing_model.rst (0 hunks)
• docs/training_methods.rst (1 hunks)
• docs/training_model.rst (1 hunks)
• evaluation/__init__.py (1 hunks)
• evaluation/deepeval_integration.py (1 hunks)
• evaluation/game_evaluation.py (3 hunks)
• evaluation/model_evaluation.py (1 hunks)
• legacy/llama31_train.py (0 hunks)
• main.py (1 hunks)
• model.dvc (1 hunks)
• mypy.ini (0 hunks)
• pyproject.toml (2 hunks)
• requirements.txt (1 hunks)
• rkllm_files/Dockerfile.rkllm (1 hunks)
• rkllm_files/entrypoint.sh (1 hunks)
• rkllm_files/rkllm_converting.py (1 hunks)
• run_pipeline.sh (3 hunks)
• testing_model/__init__.py (1 hunks)
• testing_model/__main__.py (3 hunks)
• testing_model/deepeval_func.py (0 hunks)
• testing_model/models.py (1 hunks)
• testing_model/test.py (0 hunks)
• tests/README.md (1 hunks)
• tests/__init__.py (1 hunks)
• tests/integration/test_training_functions.py (1 hunks)
• tests/test_requirements.txt (1 hunks)
• tests/unit/test_dpo_grpo.py (1 hunks)
• tools/dataset/analyze_dataset.py (1 hunks)
• tools/dataset/analyze_game_coverage.py (1 hunks)
• tools/dataset/find_future_tense_problems.py (1 hunks)
• tools/dataset/find_history_ending_problems.py (1 hunks)
• tools/dataset/find_question_action_problems.py (1 hunks)
• tools/dataset/find_threat_question_problems.py (1 hunks)
• tools/dataset/validate_extended_dataset.py (1 hunks)
• training_model/__init__.py (1 hunks)
• training_model/__main__.py (3 hunks)
• training_model/config_validation.py (1 hunks)
• training_model/data_preparation.py (1 hunks)
• training_model/dpo_train.py (1 hunks)
• training_model/exceptions.py (1 hunks)
• training_model/grpo_train.py (5 hunks)
• training_model/logging_config.py (2 hunks)
• training_model/logging_utils.py (1 hunks)
• training_model/memory_utils.py (1 hunks)

💤 Files with no reviewable changes (10)

• data/daedalus_eng.json
• data/values.txt
• mypy.ini
• .flake8
• data/test_eng.json
• docs/testing_model.rst
• testing_model/test.py
• data/test_ru.json
• legacy/llama31_train.py
• testing_model/deepeval_func.py

🧰 Additional context used

🧬 Code graph analysis (16)

main.py (1)

training_model/__init__.py (1)

• LLMLoRaCLI (32-546)

training_model/config_validation.py (2)

training_model/dpo_train.py (1)

• validate_dpo_config (19-52)

training_model/grpo_train.py (1)

• validate_grpo_config (66-96)

dev.py (1)

training_model/logging_config.py (1)

• format (59-70)

run_pipeline.sh (1)

training_model/__init__.py (1)

• pipeline (372-424)

training_model/logging_utils.py (1)

training_model/logging_config.py (1)

• log_dict (134-158)

tests/integration/test_training_functions.py (1)

training_model/one_file_train.py (8)

• change_dir (169-186)
• convert_to_gguf (1266-1331)
• data_preparation (447-559)
• generate_and_tokenize_prompt (244-280)
• generate_prompt (189-216)
• model_merge_for_converting (562-634)
• quantize_model (1680-1735)
• tokenize (219-241)

training_model/__init__.py (7)

evaluation/model_evaluation.py (2)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)

training_model/config_validation.py (2)

• get_validation_summary (434-468)
• validate_complete_config (393-431)

training_model/logging_config.py (1)

• configure_logging (73-119)

training_model/one_file_train.py (3)

• convert_to_gguf (1266-1331)
• convert_to_rkllm (1334-1511)
• train (934-1171)

training_model/optuna.py (1)

• optuna_optimize (111-190)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

evaluation/deepeval_integration.py (3)

• test_game_context_appropriateness (304-355)
• test_russian_language_quality (279-301)
• test_unintended_answer_mention (235-276)

training_model/grpo_train.py (5)

training_model/logging_utils.py (3)

• get_report_to_backend (11-33)
• log_dataset_samples (263-350)
• mlflow_phase_run (37-78)

training_model/memory_utils.py (2)

• comprehensive_memory_cleanup (55-107)
• log_memory_usage (35-52)

training_model/optimizer_factory.py (2)

• create_optimizer (154-173)
• get_optimizer_config_updates (176-197)

training_model/utils.py (1)

• get_generation_config (17-92)

training_model/config_validation.py (1)

• validate_grpo_config (242-308)

training_model/data_preparation.py (1)

evaluation/model_evaluation.py (1)

• get_user_prompt (32-61)

training_model/dpo_train.py (4)

training_model/logging_utils.py (3)

• get_report_to_backend (11-33)
• log_dataset_samples (263-350)
• mlflow_phase_run (37-78)

training_model/memory_utils.py (2)

• comprehensive_memory_cleanup (55-107)
• log_memory_usage (35-52)

training_model/optimizer_factory.py (2)

• create_optimizer (154-173)
• get_optimizer_config_updates (176-197)

training_model/one_file_train.py (1)

• train (934-1171)

testing_model/__main__.py (5)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

evaluation/model_evaluation.py (3)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)
• success_rate (297-298)

training_model/logging_config.py (1)

• configure_logging (73-119)

evaluation/deepeval_integration.py (7)

• test_game_context_appropriateness (304-355)
• test_game_context_appropriateness_async (436-484)
• test_russian_language_quality (279-301)
• test_russian_language_quality_async (400-433)
• test_unintended_answer_mention (235-276)
• test_unintended_answer_mention_async (358-397)
• _get_evaluation_model (134-162)

training_model/__init__.py (1)

• test (253-370)

testing_model/__init__.py (3)

evaluation/deepeval_integration.py (2)

• test_from_dataset (558-619)
• test_mention_number_of_values (194-232)

evaluation/model_evaluation.py (2)

• test_llm (904-969)
• test_via_llamacpp (1054-1144)

testing_model/models.py (3)

• CustomLocalModel (12-85)
• CustomMistralModel (88-198)
• CustomOpenAIModel (201-294)

training_model/__main__.py (4)

evaluation/model_evaluation.py (2)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)

training_model/logging_config.py (1)

• configure_logging (73-119)

training_model/one_file_train.py (1)

• main_train (1934-1976)

training_model/optuna.py (1)

• optuna_optimize (111-190)

evaluation/model_evaluation.py (4)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

training_model/data_preparation.py (1)

• get_user_prompt (12-23)

testing_model/models.py (3)

• generate (53-63)
• generate (146-162)
• generate (248-263)

training_model/__init__.py (1)

• test (253-370)

evaluation/deepeval_integration.py (2)

testing_model/models.py (3)

• CustomLocalModel (12-85)
• CustomMistralModel (88-198)
• CustomOpenAIModel (201-294)

training_model/utils.py (1)

• _load_environment_if_needed (95-103)

tests/unit/test_dpo_grpo.py (2)

training_model/dpo_train.py (3)

• dpo_train (141-305)
• prepare_dpo_data (55-138)
• validate_dpo_config (19-52)

training_model/grpo_train.py (8)

• grpo_train (556-863)
• debug_reward_function (99-146)
• prepare_grpo_data (444-553)
• reward_function (149-233)
• validate_grpo_config (66-96)
• reward_function_json_validation (236-289)
• reward_function_action_extraction (292-400)
• reward_function_combined (403-441)

🪛 Checkov (3.2.334)

rkllm_files/Dockerfile.rkllm

[high] 19-19: Ensure that certificate validation isn't disabled with the pip '--trusted-host' option

(CKV2_DOCKER_4)

🪛 LanguageTool

docs/README_RU.md

[uncategorized] ~173-~173: Перед знаками препинания не должно быть пробела: «конфигурация:»
Context: ...е файлы: - conf/config.yaml: Основная конфигурация - conf/model/: Настройки моделей - conf/training/: Г...

(Punct_and_spaces)

---

[uncategorized] ~174-~174: Перед знаками препинания не должно быть пробела: «моделей:»
Context: ...конфигурация - conf/model/: Настройки моделей - conf/training/: Гиперпараметры обучения ### Пример кон...

(Punct_and_spaces)

docs/CONTRIBUTING.md

[grammar] ~262-~262: Use a hyphen to join words.
Context: ...tion Style - Docstrings: Use Google style docstrings - Comments: Explain...

(QB_NEW_EN_HYPHEN)

---

[uncategorized] ~443-~443: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...king changes - MINOR: New features, backwards compatible - PATCH: Bug fixes, backwards compa...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🪛 markdownlint-cli2 (0.18.1)

docs/README_RU.md

3-3: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

283-283: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

330-330: Bare URL used

(MD034, no-bare-urls)

---

349-349: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

358-358: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

376-376: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

389-389: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

399-399: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

415-415: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

427-427: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

444-444: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

docs/CONTRIBUTING.md

293-293: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

395-395: Bare URL used

(MD034, no-bare-urls)

---

464-464: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

README.md

5-5: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

203-203: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

211-211: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

346-346: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

450-450: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

459-459: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

477-477: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

490-490: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

500-500: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

516-516: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

528-528: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

545-545: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🪛 Ruff (0.14.2)

tools/dataset/find_question_action_problems.py

47-47: Loop control variable reason not used within loop body

(B007)

training_model/config_validation.py

169-169: Do not catch blind exception: Exception

(BLE001)

---

181-181: Redundant exception object included in logging.exception call

(TRY401)

---

424-424: Redundant exception object included in logging.exception call

(TRY401)

---

464-464: Do not catch blind exception: Exception

(BLE001)

dev.py

1-1: Shebang is present but file is not executable

(EXE001)

---

40-40: Do not catch blind exception: Exception

(BLE001)

---

97-97: subprocess call: check for execution of untrusted input

(S603)

---

339-339: Abstract raise to an inner function

(TRY301)

---

339-339: Avoid specifying long messages outside the exception class

(TRY003)

---

340-340: subprocess call: check for execution of untrusted input

(S603)

---

351-351: Abstract raise to an inner function

(TRY301)

---

351-351: Avoid specifying long messages outside the exception class

(TRY003)

---

353-353: subprocess call: check for execution of untrusted input

(S603)

---

377-377: Do not catch blind exception: Exception

(BLE001)

---

390-390: Do not catch blind exception: Exception

(BLE001)

---

396-396: Do not catch blind exception: Exception

(BLE001)

training_model/memory_utils.py

139-139: Do not catch blind exception: Exception

(BLE001)

training_model/logging_utils.py

70-71: Remove exception handler; error is immediately re-raised

(TRY203)

---

103-103: Do not catch blind exception: Exception

(BLE001)

---

144-144: Do not catch blind exception: Exception

(BLE001)

---

188-188: Do not catch blind exception: Exception

(BLE001)

---

222-222: Do not catch blind exception: Exception

(BLE001)

---

256-256: Consider moving this statement to an else block

(TRY300)

---

259-259: Redundant exception object included in logging.exception call

(TRY401)

---

349-349: Do not catch blind exception: Exception

(BLE001)

tests/integration/test_training_functions.py

58-58: Unused function argument: tokenize

(ARG001)

rkllm_files/rkllm_converting.py

1-1: Shebang is present but file is not executable

(EXE001)

---

114-114: Avoid specifying long messages outside the exception class

(TRY003)

---

190-190: Consider moving this statement to an else block

(TRY300)

---

193-193: Redundant exception object included in logging.exception call

(TRY401)

training_model/logging_config.py

93-93: Avoid specifying long messages outside the exception class

(TRY003)

---

97-97: Avoid specifying long messages outside the exception class

(TRY003)

training_model/__init__.py

5-5: Unused noqa directive (non-enabled: F401)

Remove unused noqa directive

(RUF100)

---

116-116: Avoid specifying long messages outside the exception class

(TRY003)

---

160-160: Avoid specifying long messages outside the exception class

(TRY003)

---

330-330: Unused function argument: correct_answer

(ARG001)

---

330-330: Unused function argument: kwargs

(ARG001)

---

335-335: Unused function argument: correct_answer

(ARG001)

---

405-405: Avoid specifying long messages outside the exception class

(TRY003)

testing_model/models.py

25-25: Unused method argument: args

(ARG002)

---

26-26: Unused method argument: kwargs

(ARG002)

---

105-105: Unused method argument: args

(ARG002)

---

106-106: Unused method argument: kwargs

(ARG002)

---

219-219: Unused method argument: args

(ARG002)

---

220-220: Unused method argument: kwargs

(ARG002)

tools/dataset/analyze_dataset.py

59-59: Loop control variable key not used within loop body

Rename unused key to _key

(B007)

training_model/grpo_train.py

226-229: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

236-236: Unused function argument: kwargs

(ARG001)

---

285-285: Do not catch blind exception: Exception

(BLE001)

---

286-286: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

396-396: Do not catch blind exception: Exception

(BLE001)

---

397-397: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

579-579: Avoid specifying long messages outside the exception class

(TRY003)

---

672-672: Avoid specifying long messages outside the exception class

(TRY003)

training_model/dpo_train.py

165-165: Avoid specifying long messages outside the exception class

(TRY003)

---

193-193: Avoid specifying long messages outside the exception class

(TRY003)

testing_model/__main__.py

87-89: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

90-92: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

96-96: Unused function argument: correct_answer

(ARG001)

---

96-96: Unused function argument: kwargs

(ARG001)

---

108-110: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

111-113: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

117-117: Unused function argument: correct_answer

(ARG001)

---

139-141: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

142-144: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

evaluation/model_evaluation.py

49-49: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

49-49: String contains ambiguous о (CYRILLIC SMALL LETTER O). Did you mean o (LATIN SMALL LETTER O)?

(RUF001)

---

49-49: String contains ambiguous р (CYRILLIC SMALL LETTER ER). Did you mean p (LATIN SMALL LETTER P)?

(RUF001)

---

54-54: String contains ambiguous Т (CYRILLIC CAPITAL LETTER TE). Did you mean T (LATIN CAPITAL LETTER T)?

(RUF001)

---

58-58: String contains ambiguous О (CYRILLIC CAPITAL LETTER O). Did you mean O (LATIN CAPITAL LETTER O)?

(RUF001)

---

58-58: String contains ambiguous е (CYRILLIC SMALL LETTER IE). Did you mean e (LATIN SMALL LETTER E)?

(RUF001)

---

275-275: Do not catch blind exception: Exception

(BLE001)

---

558-558: Redundant exception object included in logging.exception call

(TRY401)

---

612-612: Unused noqa directive (non-enabled: SIM113)

Remove unused noqa directive

(RUF100)

---

618-618: Redundant exception object included in logging.exception call

(TRY401)

---

699-699: Abstract raise to an inner function

(TRY301)

---

711-711: Do not catch blind exception: Exception

(BLE001)

---

734-734: Unpacked variable score is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

---

738-738: Abstract raise to an inner function

(TRY301)

---

751-751: Do not catch blind exception: Exception

(BLE001)

---

773-773: Redundant exception object included in logging.exception call

(TRY401)

---

828-828: Unused noqa directive (non-enabled: SIM113)

Remove unused noqa directive

(RUF100)

---

963-963: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

964-964: Redundant exception object included in logging.exception call

(TRY401)

---

985-985: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

992-992: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1066-1066: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1071-1071: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1114-1114: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)

---

1115-1115: String contains ambiguous У (CYRILLIC CAPITAL LETTER U). Did you mean Y (LATIN CAPITAL LETTER Y)?

(RUF001)

---

1116-1116: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

1117-1117: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

evaluation/game_evaluation.py

7-7: Unused function argument: kwargs

(ARG001)

---

44-44: Avoid specifying long messages outside the exception class

(TRY003)

evaluation/deepeval_integration.py

30-30: Do not catch blind exception: Exception

(BLE001)

---

44-44: Do not catch blind exception: Exception

(BLE001)

---

71-71: Avoid specifying long messages outside the exception class

(TRY003)

---

95-95: Unused function argument: cfg

(ARG001)

---

99-99: Avoid specifying long messages outside the exception class

(TRY003)

---

162-162: Avoid specifying long messages outside the exception class

(TRY003)

---

184-184: subprocess call: check for execution of untrusted input

(S603)

---

219-219: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

236-236: Unused function argument: expected_answer

(ARG001)

---

263-263: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

359-359: Unused function argument: expected_answer

(ARG001)

---

383-383: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

541-541: Do not catch blind exception: Exception

(BLE001)

---

544-544: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

560-560: Unused function argument: test_dataset

(ARG001)

tests/unit/test_dpo_grpo.py

632-632: Unused method argument: mock_dpo_config

(ARG002)

---

700-700: Unused method argument: mock_grpo_config

(ARG002)

🪛 Shellcheck (0.11.0)

run_pipeline.sh

[warning] 76-76: Quote this to prevent word splitting.

(SC2046)

---

[warning] 77-77: Quote this to prevent word splitting.

(SC2046)

---

[error] 124-124: Parsing stopped here. Is this keyword correctly matched up?

(SC1089)

🔇 Additional comments (23)

training_model/exceptions.py (1)

1-14: LGTM! Well-structured exception hierarchy.

The module docstring clearly explains the purpose, and the base exception class LLMLoRaError provides a solid foundation for catching all framework-specific errors. This design enables better error diagnosis and handling compared to generic exceptions.

model.dvc (1)

1-6: LGTM!

The DVC tracking configuration for the model directory is correctly structured with appropriate hash type and metadata.

.dvc/.gitignore (1)

1-3: LGTM!

Standard DVC gitignore entries appropriately exclude local configurations and ephemeral data from version control.

evaluation/__init__.py (1)

1-1: LGTM!

Standard package initialization with clear documentation.

data.dvc (1)

1-6: LGTM!

The DVC tracking configuration for the data directory is correctly structured with appropriate metadata.

.dvc/config (2)

1-3: LGTM on cache configuration.

The hardlink cache type with protection enabled is an appropriate choice for performance and data integrity.

---

6-7: Remove the suggested fix; the approach using ${DVC_REMOTE_URL} environment variables in .dvc/config is not supported by DVC and will not work.

The portability concern is valid — UNC paths will not work natively on Linux and require the share to be mounted to a POSIX path. However, .dvc/config is meant to be committed (per DVC best practices for non-sensitive shared settings); the real issue is that different machines may need different URLs or mount paths.

The correct solution is to move the remote configuration to .dvc/config.local (which DVC gitignores by default), allowing each developer to configure their own URL based on their environment:

dvc remote add nas-remote //diskstation_105/dvc_storage --local
# or on Linux with the share mounted:
dvc remote add nas-remote /mnt/dvc_storage --local

Each machine then maintains its own .dvc/config.local without committing it. This respects DVC's design where .dvc/config holds shared settings and .dvc/config.local holds machine-specific ones.

Likely an incorrect or invalid review comment.

evaluation/game_evaluation.py (1)

1-44: LGTM! Past review feedback addressed.

The refactoring successfully addresses the previous review comment about the missing error message. The new _raise_action_mismatch() helper provides a clear, descriptive error message when actions don't match. The **kwargs parameter enables flexible integration with testing frameworks while maintaining backward compatibility.

requirements.txt (1)

1-203: LGTM: Dependency updates align with CUDA 12.4 upgrade.

The CUDA version update from cu118 to cu124 and associated dependency version bumps are consistent with the project's modernization goals described in the README.

pyproject.toml (1)

1-138: LGTM: Dependency and tooling updates support project modernization.

The updates include:

• Broader Python version support (3.11-3.13)
• CUDA 12.4 support
• Loosened dependency constraints for flexibility
• Enhanced code quality tooling (ruff, bandit, codespell)
• Comprehensive pytest configuration

These changes align well with the comprehensive refactoring described in the PR.

conf/config.yaml (1)

1-259: LGTM: Configuration expansion supports new features.

The configuration updates comprehensively support the new features mentioned in the PR:

• GRPO and DPO training methods
• RKLLM conversion for edge deployment
• Enhanced memory management
• Optuna hyperparameter optimization
• DeepEval integration

The structure is well-organized with clear comments and sensible defaults.

.pre-commit-config.yaml (1)

1-72: LGTM: Pre-commit hooks modernized and enhanced.

The updates improve code quality tooling:

• Version updates for core hooks
• Flake8 replaced with more comprehensive ruff configuration
• Bandit added for security checks
• Codespell added for spelling checks
• Better hook descriptions and configurations

These changes align with the tooling updates in pyproject.toml.

tests/__init__.py (1)

1-2: LGTM!

Standard package marker file for the tests directory.

docs/docker_deployment.rst (1)

1-912: Comprehensive Docker deployment documentation.

This extensive documentation provides excellent coverage of Docker-based deployment workflows, including Ollama integration, security considerations, scaling strategies, and monitoring. The examples are well-structured and cover multiple deployment scenarios.

Consider these minor enhancements for future iterations:

• Add version pinning examples in the Docker Compose snippets to improve reproducibility
• Include troubleshooting section for common deployment issues
• Add links to external documentation (Docker, Ollama, Kubernetes) for readers who need additional context

main.py (1)

1-7: Clean Fire-based CLI entrypoint.

The migration to Python Fire for CLI handling is a good improvement for usability. The implementation is straightforward and follows Fire's best practices.

docs/index.rst (1)

1-109: Excellent documentation structure.

The index provides a clear, comprehensive overview of the framework with well-organized sections. The feature highlights, architecture overview, and structured navigation make it easy for users to find what they need.

tests/README.md (1)

1-142: Comprehensive testing guide.

Excellent documentation covering test execution, structure, debugging, and contribution guidelines. The examples and fixture descriptions will help developers understand and extend the test suite effectively.

tests/test_requirements.txt (1)

1-22: Well-organized test requirements.

The test requirements file is clearly structured with appropriate comments explaining the purpose of each section and noting dependencies that overlap with main requirements.

testing_model/__init__.py (1)

1-16: LGTM! Clean API reorganization.

The imports and public API surface are well-structured. The module correctly exposes testing utilities and custom model classes from their new locations.

tests/integration/test_training_functions.py (1)

116-330: Excellent test coverage for training pipeline functions.

The test suite comprehensively covers key training functions including directory management, prompt generation, tokenization, data preparation, model merging, and quantization. Good use of mocking to isolate dependencies.

testing_model/__main__.py (2)

31-161: Well-designed DeepEval integration with dynamic metric loading.

The _create_deepeval_test_functions helper cleanly handles:

• Configuration-based metric selection
• Safe fallbacks when DeepEval is unavailable
• Clear logging of enabled/disabled metrics
• Proper wrapper creation for signature compatibility

---

228-254: Comprehensive test summary logging.

The final summary provides detailed metrics per test function with clear formatting. Good use of conditional logic to handle both single and multiple test scenarios.

training_model/__init__.py (1)

32-547: Excellent CLI design with comprehensive training pipeline integration.

The LLMLoRaCLI class provides a clean Fire-based interface for:

• Training with optional Optuna optimization
• Multi-format model conversion (GGUF/RKLLM)
• Testing with optional DeepEval metrics
• Full pipeline orchestration

The configuration handling with Hydra integration, override application, and validation checks is well-implemented.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Make file executable to match shebang.

The file has a shebang but isn't marked as executable.

Run this command to make it executable:

chmod +x dev.py

🧰 Tools

🪛 Ruff (0.14.2)

1-1: Shebang is present but file is not executable

(EXE001)

🤖 Prompt for AI Agents

In dev.py around lines 1 to 1 the file contains a shebang but is not executable;
make the script executable by setting the execute bit for appropriate users
(e.g., run chmod +x dev.py in the repo or adjust file mode in CI/git so the file
is committed with executable permissions) so the shebang is honored when running
the script directly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Ensure llm_training can reach ollama.

llm_training is isolated on mlflow-network, while ollama stays on the default network, so DNS for ollama is unreachable and the training container can’t hit http://ollama:11434.

Attach both services to the same networks, e.g.:

   ollama:
     ...
-    volumes:
-      - ollama_models:/root/.ollama
-      - ./models:/app/models
+    volumes:
+      - ollama_models:/root/.ollama
+      - ./models:/app/models
     environment:
       - OLLAMA_MODELS=/app/models
     runtime: nvidia
+    networks:
+      - default
+      - mlflow-network
   ...
   llm_training:
     ...
-    networks:
-      - mlflow-network
+    networks:
+      - default
+      - mlflow-network

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- "11434:11434"
	volumes:
	- ollama_models:/root/.ollama
	- ./models:/training_model/models
	- ./models:/app/models
	environment:
	- OLLAMA_MODELS=/training_model/models
	- OLLAMA_MODELS=/app/models
	runtime: nvidia
	healthcheck:
	test: [ "CMD", "curl", "-f", "http://localhost:11434" ]
	interval: 10s
	timeout: 5s
	retries: 5
	rkllm_converter:
	build:
	context: rkllm_files/
	dockerfile: Dockerfile.rkllm
	image: rkllm_converter
	container_name: rkllm_converter
	volumes:
	- ./models:/app/models
	- ./data:/app/data
	# Temporary volumes for conversion I/O
	- /tmp/rkllm_input:/input
	- /tmp/rkllm_output:/output
	environment:
	- PYTHONUNBUFFERED=1
	profiles: ["converter"]
	llm_training:
	build:
	context: .
	dockerfile: Dockerfile
	container_name: llm_training
	volumes:
	- ./models:/training_model/models
	- ./data:/training_model/data
	- ./conf:/training_model/conf
	- ./models:/app/models
	- ./data:/app/data
	- ./conf:/app/conf
	- /var/run/docker.sock:/var/run/docker.sock
	depends_on:
	ollama:
	condition: service_healthy
	environment:
	- OLLAMA_HOST=http://ollama:11434
	runtime: nvidia
	networks:
	- mlflow-network
	- "11434:11434"
	volumes:
	- ollama_models:/root/.ollama
	- ./models:/app/models
	environment:
	- OLLAMA_MODELS=/app/models
	runtime: nvidia
	networks:
	- default
	- mlflow-network
	healthcheck:
	test: [ "CMD", "curl", "-f", "http://localhost:11434" ]
	interval: 10s
	timeout: 5s
	retries: 5
	rkllm_converter:
	build:
	context: rkllm_files/
	dockerfile: Dockerfile.rkllm
	image: rkllm_converter
	container_name: rkllm_converter
	volumes:
	- ./models:/app/models
	- ./data:/app/data
	# Temporary volumes for conversion I/O
	- /tmp/rkllm_input:/input
	- /tmp/rkllm_output:/output
	environment:
	- PYTHONUNBUFFERED=1
	profiles: ["converter"]
	llm_training:
	build:
	context: .
	dockerfile: Dockerfile
	container_name: llm_training
	volumes:
	- ./models:/app/models
	- ./data:/app/data
	- ./conf:/app/conf
	- /var/run/docker.sock:/var/run/docker.sock
	depends_on:
	ollama:
	condition: service_healthy
	environment:
	- OLLAMA_HOST=http://ollama:11434
	runtime: nvidia
	networks:
	- default
	- mlflow-network

🤖 Prompt for AI Agents

docker-compose.yaml around lines 8-56: llm_training is placed on mlflow-network
while ollama remains only on the default network, so the llm_training service
cannot resolve the hostname "ollama"; modify the compose file to attach ollama
to the same network(s) as llm_training (e.g., add mlflow-network under the
ollama service networks section, or add both services to a shared network) and
ensure the mlflow-network is declared under the top-level networks section so
Docker Compose creates it.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Overly permissive directory permissions pose a security risk.

Setting permissions to 777 grants read, write, and execute access to all users, which violates the principle of least privilege and could allow unauthorized modifications.

Apply this diff to use more restrictive permissions:

-RUN mkdir -p /app/models && chmod 777 /app/models
+RUN mkdir -p /app/models && chmod 755 /app/models

If write access by the Ollama process is required, consider using 775 with appropriate group ownership instead of world-writable permissions.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	RUN mkdir -p /app/models && chmod 777 /app/models
	RUN mkdir -p /app/models && chmod 755 /app/models

🤖 Prompt for AI Agents

In Dockerfile_ollama around line 6, the RUN command currently sets overly
permissive 777 on /app/models; change this to create the directory, set
ownership to the user/group that the Ollama process runs as, and use restrictive
permissions (e.g. 775) so only owner/group can write while others are
read/execute. Ensure the Dockerfile uses chown to assign the directory to the
Ollama runtime user/group before or when setting permissions and avoid
world-writable mode.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove duplicate directive and verify class references.

Two issues:

1. Line 283 has a duplicate automethod directive for convert
2. References to main.LLMLoRAFramework (lines 280-281, 284-285) may be incorrect - should verify if the class is actually training_model.LLMLoRaCLI based on the context from other files in this PR

Remove the duplicate on line 283 and verify the class name matches the actual implementation:

 .. automethod:: main.LLMLoRAFramework.train_model
 .. automethod:: main.LLMLoRAFramework.evaluate_model
 .. automethod:: training_model.LLMLoRaCLI.convert
-.. automethod:: training_model.LLMLoRaCLI.convert
 .. automethod:: main.LLMLoRAFramework.list_models
 .. automethod:: main.LLMLoRAFramework.clean_checkpoints

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	.. automethod:: main.LLMLoRAFramework.train_model
	.. automethod:: main.LLMLoRAFramework.evaluate_model
	.. automethod:: training_model.LLMLoRaCLI.convert
	.. automethod:: training_model.LLMLoRaCLI.convert
	.. automethod:: main.LLMLoRAFramework.list_models
	.. automethod:: main.LLMLoRAFramework.clean_checkpoints
	.. automethod:: main.LLMLoRAFramework.train_model
	.. automethod:: main.LLMLoRAFramework.evaluate_model
	.. automethod:: training_model.LLMLoRaCLI.convert
	.. automethod:: main.LLMLoRAFramework.list_models
	.. automethod:: main.LLMLoRAFramework.clean_checkpoints

🤖 Prompt for AI Agents

In docs/api_reference.rst around lines 280 to 286 there is a duplicate
automethod directive for training_model.LLMLoRaCLI.convert (line 283) and
several automethods reference main.LLMLoRAFramework (lines 280-281 and 284-285)
which may be incorrect; remove the duplicated automethod entry for convert and
update the referenced class paths to the actual implementing class (likely
training_model.LLMLoRaCLI) by replacing any incorrect main.LLMLoRAFramework
prefixes with the correct module/class name after verifying the implementation
in the source files.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix typo: "ROGUE" should be "ROUGE"

The metric name is misspelled. It should be "ROUGE Score" (Recall-Oriented Understudy for Gisting Evaluation), not "ROGUE Score".

Apply this diff:

-* **ROGUE Score**: Recall-oriented evaluation for summarization
+* **ROUGE Score**: Recall-oriented evaluation for summarization

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	* **BLEU Score**: Bilingual evaluation for text similarity
	* **ROGUE Score**: Recall-oriented evaluation for summarization
	* **BLEU Score**: Bilingual evaluation for text similarity
	* **ROUGE Score**: Recall-oriented evaluation for summarization

🤖 Prompt for AI Agents

In docs/evaluation.rst around lines 95 to 96, the metric name "ROGUE Score" is
misspelled; change "ROGUE Score" to "ROUGE Score" so the line reads "* **ROUGE
Score**: Recall-oriented evaluation for summarization". Ensure only the spelling
is corrected and capitalization remains consistent with the surrounding list.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Rename function to avoid confusion with evaluation.model_evaluation.get_user_prompt.

This module's get_user_prompt(data: str) has the same name but a different signature from evaluation.model_evaluation.get_user_prompt(data: dict[str, Any]). This creates confusion and makes the conditional import at line 50 necessary.

Apply this diff to use a more descriptive name:

-def get_user_prompt(data: str) -> str:
+def get_classic_user_prompt(data: str) -> str:
     """
     Construct a user prompt from conversation data.
 
     Args:
         data (str): conversation data
 
     Returns:
         str: Formatted prompt string with conversation context.
     """
     prompt = f"Ответь на вопрос пользователя коротко: {data}"
     return prompt

Then update the call at line 70:

         else:
             # Classic method: simple instruction/output format
             instruction = str(example.get("instruction", ""))
-            user_message = get_user_prompt(instruction)
+            user_message = get_classic_user_prompt(instruction)
             bot_message = str(example.get("output", ""))

🤖 Prompt for AI Agents

In training_model/data_preparation.py around lines 12 to 23, the function
get_user_prompt(data: str) conflicts with
evaluation.model_evaluation.get_user_prompt (different signature) and causes the
conditional import; rename this function to a more descriptive name (e.g.,
build_user_prompt_from_text or format_user_prompt_text) and update any internal
references accordingly; then update the call at line 70 to use the new name so
the module no longer collides with evaluation.model_evaluation.get_user_prompt.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove duplicate logging import.

logging is already imported at line 6, no need to import it again in the function.

Apply this diff:

 def do_transform() -> None:
     """One run function to convert the dataset."""
-    import logging
-
     logger = logging.getLogger(__name__)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def do_transform() -> None:
	"""One run function to convert the dataset."""
	import logging
	logger = logging.getLogger(__name__)
	base_dir = Path(__file__).resolve().parent.parent
	input_file_path = base_dir / "data" / "intents_dataset.json"
	with input_file_path.open("r", encoding="utf-8") as f:
	input_json = json.load(f)
	result = transform_topics(input_json)
	output_file_path = base_dir / "data" / "intent_responses.json"
	with output_file_path.open("w", encoding="utf-8") as f:
	json.dump(result, f, ensure_ascii=False, indent=4)
	logger.info(
	"Dataset successfully converted to new format and saved to %s",
	output_file_path,
	)
	def do_transform() -> None:
	"""One run function to convert the dataset."""
	logger = logging.getLogger(__name__)
	base_dir = Path(__file__).resolve().parent.parent
	input_file_path = base_dir / "data" / "intents_dataset.json"
	with input_file_path.open("r", encoding="utf-8") as f:
	input_json = json.load(f)
	result = transform_topics(input_json)
	output_file_path = base_dir / "data" / "intent_responses.json"
	with output_file_path.open("w", encoding="utf-8") as f:
	json.dump(result, f, ensure_ascii=False, indent=4)
	logger.info(
	"Dataset successfully converted to new format and saved to %s",
	output_file_path,
	)

🤖 Prompt for AI Agents

In training_model/data_preparation.py around lines 113 to 135 there is a
redundant "import logging" inside do_transform(); remove that inner import so
the function uses the module-level logging import (already present at the top of
the file) and keep the logger = logging.getLogger(__name__) and remaining code
unchanged.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix Hydra data-directory resolution.

Hydra changes cwd to the run output directory, so joining cfg.paths.data_dir with Path.cwd() looks under outputs/.../data and makes validation/preparation fail even though the dataset exists at the project root. This breaks normal Hydra runs and the new tests. Please resolve the base directory via hydra.utils.get_original_cwd() (with a safe fallback) before computing data_dir.

-from pathlib import Path
+from pathlib import Path
+
+from hydra.utils import get_original_cwd
@@
-    data_dir = Path.cwd() / cfg.paths.data_dir
+    try:
+        data_root = Path(get_original_cwd())
+    except ValueError:
+        data_root = Path.cwd()
+    data_dir = data_root / cfg.paths.data_dir
@@
-    data_dir = Path.cwd() / cfg.paths.data_dir
+    try:
+        data_root = Path(get_original_cwd())
+    except ValueError:
+        data_root = Path.cwd()
+    data_dir = data_root / cfg.paths.data_dir

🤖 Prompt for AI Agents

In training_model/dpo_train.py around lines 3 to 71, Hydra changes cwd to the
run output dir so using Path.cwd() to resolve cfg.paths.data_dir is wrong;
replace Path.cwd() with hydra.utils.get_original_cwd() (import it at the top)
and fall back to Path.cwd() if get_original_cwd() is not available or returns
None/empty; apply this change where data_dir is computed both in
validate_dpo_config and prepare_dpo_data so data files are resolved relative to
the project root rather than the Hydra output dir.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Defend against missing GRPO config sections before touching them.

This reimplementation assumes cfg.grpo and cfg.paths already exist. When either section is omitted or optional in the Hydra config, validate_grpo_config(cfg) now raises AttributeError instead of producing a controlled error message, which blocks GRPO outright. The previous shared validator in training_model/config_validation.py (lines 241-307 in the provided snippet) handled these edge cases gracefully—let’s bring those guards back locally or, better yet, reuse the shared helper.

Please add explicit checks before dereferencing these sections, e.g.:

+    if not hasattr(cfg, "grpo") or cfg.grpo is None:
+        logging.error("GRPO configuration not found - aborting validation")
+        return False
+
+    if not hasattr(cfg, "paths") or cfg.paths is None:
+        logging.error("paths section is required for GRPO data files")
+        return False
+
+    data_dir_value = getattr(cfg.paths, "data_dir", None)
+    if data_dir_value is None:
+        logging.error("paths.data_dir must be defined for GRPO")
+        return False

and then proceed to build the dataset paths with data_dir_value.

🤖 Prompt for AI Agents

In training_model/grpo_train.py around lines 75 to 95, the validator
dereferences cfg.grpo and cfg.paths without checking they exist which causes
AttributeError when those sections are omitted; add guards to detect missing
sections (e.g., if not hasattr(cfg, "grpo") or not hasattr(cfg, "paths") then
log a clear error and return False), or preferably import and call the shared
validator from training_model/config_validation.py to reuse its existing checks;
after confirming cfg.paths and cfg.grpo exist, read data_dir_value =
getattr(cfg.paths, "data_dir") (or similar) and construct train_file/val_file
from that value before checking .exists(), keeping the same error logs and
return False behaviour on missing params or files.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Read generation_config like a dict, otherwise overrides never apply.

get_generation_config (see training_model/utils.py) returns a plain dict. Using getattr(generation_config, "temperature", …) always misses and falls back to the default literal, so any GRPO-specific overrides in the config file are ignored during training. This regresses the configurability of GRPOConfig.

Switch these lookups to dict.get so that user-specified values make it into the trainer:

-        temperature=getattr(generation_config, "temperature", 1.0),
-        top_p=getattr(generation_config, "top_p", 1.0),
-        top_k=getattr(generation_config, "top_k", 50),
+        temperature=generation_config.get("temperature", 1.0),
+        top_p=generation_config.get("top_p", 1.0),
+        top_k=generation_config.get("top_k", 50),

This keeps GRPO aligned with the auxiliary generation settings surfaced in the new config flow and the TRL GRPOConfig API. (huggingface.co)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	temperature=getattr(generation_config, "temperature", 1.0),
	top_p=getattr(generation_config, "top_p", 1.0),
	top_k=getattr(generation_config, "top_k", 50),
	temperature=generation_config.get("temperature", 1.0),
	top_p=generation_config.get("top_p", 1.0),
	top_k=generation_config.get("top_k", 50),

🤖 Prompt for AI Agents

In training_model/grpo_train.py around lines 758 to 760, the code uses
getattr(generation_config, "...", ...) but generation_config is a plain dict
from get_generation_config, so overrides are ignored; replace those getattr
calls with generation_config.get("temperature", 1.0),
generation_config.get("top_p", 1.0), and generation_config.get("top_k", 50)
respectively so user-provided values are respected while keeping the same
default fallbacks.

[coderabbitai]

Actionable comments posted: 15

♻️ Duplicate comments (30)

docs/fire_cli.rst (2)

213-215: Inconsistent command names in pipeline example.

The pipeline example uses train and test instead of the documented train_model and evaluate_model commands (lines 24-72).

---

262-280: Inconsistent command names in workflow examples.

The development and production workflow examples use shortened command names (train, test) instead of the documented full names (train_model, evaluate_model).

tools/dataset/find_history_ending_problems.py (1)

10-12: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review. The hardcoded Windows-specific path prevents cross-platform usage.

tools/dataset/find_threat_question_problems.py (1)

9-11: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review.

tools/dataset/find_question_action_problems.py (1)

9-11: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review.

tools/dataset/find_future_tense_problems.py (1)

9-11: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review.

tools/dataset/analyze_dataset.py (1)

5-7: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review.

tools/dataset/analyze_game_coverage.py (1)

4-6: Replace hardcoded Windows path with configurable path.

This issue was already flagged in a previous review.

README.md (1)

346-381: Add language identifier to fenced code block.

This issue was already flagged in a previous review. The project structure code block should specify a language (e.g., text) for proper rendering.

tools/dataset/validate_extended_dataset.py (1)

6-13: Fix hardcoded path and add robust file handling

The script still hardcodes a Windows-only absolute path and exits without guarding common I/O failures, so it breaks immediately in Docker/CI or on non-Windows hosts. Please resolve the path relative to the repo (or from an env var) and wrap the open/parse step in targeted error handling.

-import json
-from collections import Counter
-from pathlib import Path
+import json
+import os
+import sys
+from collections import Counter
+from pathlib import Path
@@
-# Load extended dataset
-data_path = Path("T:/projects/LLM_LoRa/LLM_LoRa/data/dataset_ru_extended.json")
-with open(data_path, encoding="utf-8") as f:
-    data = json.load(f)
+# Load extended dataset
+data_path = Path(
+    os.environ.get(
+        "EXTENDED_DATASET_PATH",
+        Path(__file__).resolve().parents[2] / "data" / "dataset_ru_extended.json",
+    )
+)
+
+try:
+    with data_path.open(encoding="utf-8") as f:
+        data = json.load(f)
+except FileNotFoundError:
+    print(f"Error: Dataset not found at {data_path}")
+    sys.exit(1)
+except PermissionError:
+    print(f"Error: Permission denied reading {data_path}")
+    sys.exit(1)
+except json.JSONDecodeError as exc:
+    print(f"Error: Invalid JSON in dataset: {exc}")
+    sys.exit(1)

rkllm_files/rkllm_converting.py (1)

25-27: Guard torch.cuda.empty_cache() for CPU-only environments

Calling torch.cuda.empty_cache() before checking CUDA availability still triggers AssertionError: Torch not compiled with CUDA enabled on CPU-only installs, preventing the converter from even parsing CLI arguments.(reddit.com) Add a guard (or swallow the assertion) so the tool remains usable without CUDA.

 os.environ["PYTORCH_CUDA_ALLOC_CONF"] = "expandable_segments:True"
-torch.cuda.empty_cache()
+if torch.cuda.is_available():
+    torch.cuda.empty_cache()

evaluation/model_evaluation.py (3)

66-66: Remove import-time mutation of ollama.base_url.

Setting the Ollama client base URL at import time mutates global state for every consumer of this module and makes it impossible to use different endpoints in the same process (e.g., parallel tests or multi-tenant services). Configure the URL when you create or receive the client instead (inside test_llm or before calling ollama_generate) and drop the module-level assignment.

---

167-170: Provide a meaningful error message when the LLM returns no choices.

Raising a bare RuntimeError() (and suppressing the exception chain with from None) gives zero context when response.choices is empty, making on-call debugging painful. Include a descriptive message and keep the original traceback. This mirrors the earlier review feedback.

---

711-752: Stop swallowing unexpected exceptions in async test execution.

Catching (AssertionError, Exception) collapses genuine programming errors (TypeError, AttributeError, etc.) into generic test failures, hiding bugs and contradicting Ruff/BLE001 guidance from the last review. Catch AssertionError only for assertion failures, and log & re-raise unexpected exceptions so they surface. Apply the same fix to the non-list branch.

tests/README.md (1)

12-15: Remove hardcoded Windows path.

Line 14 contains a hardcoded Windows-specific path that won't work for other developers or CI environments.

Apply this diff:

 2. Ensure you're in the project root directory:
 ```bash
-cd T:\projects\LLM_LoRa\LLM_LoRa
+cd /path/to/LLM_LoRa  # Replace with your project path

</blockquote></details>
<details>
<summary>rkllm_files/Dockerfile.rkllm (2)</summary><blockquote>

`3-3`: **Missing sources_bionic.list file will cause Docker build to fail.**

The `COPY` instruction references `sources_bionic.list` which doesn't exist in the repository, causing Docker build failures.

---

`19-21`: **Security: Certificate validation disabled for pip.**

Disabling certificate validation with `--trusted-host` exposes the build to man-in-the-middle attacks.




Apply this diff to remove insecure options:

```diff
-RUN python -m pip install --upgrade pip -i https://pypi.tuna.tsinghua.edu.cn/simple --trusted-host=pypi.tuna.tsinghua.edu.cn
+RUN python -m pip install --upgrade pip -i https://pypi.tuna.tsinghua.edu.cn/simple
 RUN pip3 config set global.index-url https://pypi.tuna.tsinghua.edu.cn/simple
-RUN pip3 config set install.trusted-host pypi.tuna.tsinghua.edu.cn

Dockerfile_ollama (1)

6-6: Overly permissive directory permissions pose a security risk.

Setting permissions to 777 violates the principle of least privilege and could allow unauthorized modifications.

Apply this diff:

-RUN mkdir -p /app/models && chmod 777 /app/models
+RUN mkdir -p /app/models && chmod 755 /app/models

docs/evaluation.rst (1)

96-96: Fix typo: "ROGUE" should be "ROUGE"

The metric name is misspelled at line 96 as well.

Apply this diff:

-* **ROGUE Score**: Recall-oriented evaluation for summarization
+* **ROUGE Score**: Recall-oriented evaluation for summarization

docs/training_methods.rst (1)

408-411: Incorrect line continuation in bash example.

The bash command uses double backslashes (\\) instead of single backslashes (\) for line continuation.

Apply this diff:

     # Evaluate preference-trained model
-    python main.py evaluate_model \\
-        --model_path=models/dpo_model \\
-        --evaluation_type=preference \\
+    python main.py evaluate_model \
+        --model_path=models/dpo_model \
+        --evaluation_type=preference \
         --metrics=["preference_accuracy", "alignment_score"]

docs/api_reference.rst (1)

280-286: Fix duplicate and rewrite documentation to reference actual implementation.

The documentation has two critical issues:

1. Duplicate on line 283: .. automethod:: training_model.LLMLoRaCLI.convert appears twice consecutively.

2. Non-existent class references: Lines 280-281 and 284-285 reference main.LLMLoRAFramework with methods train_model, evaluate_model, list_models, and clean_checkpoints. This class does not exist in the codebase. Only training_model.LLMLoRaCLI exists in the project.

Replace with the actual public methods from LLMLoRaCLI:

 .. automethod:: training_model.LLMLoRaCLI.train
 .. automethod:: training_model.LLMLoRaCLI.optimize
 .. automethod:: training_model.LLMLoRaCLI.convert
-.. automethod:: training_model.LLMLoRaCLI.convert
 .. automethod:: training_model.LLMLoRaCLI.test
 .. automethod:: training_model.LLMLoRaCLI.pipeline

training_model/data_preparation.py (1)

113-118: Drop the redundant import

There’s still a second import logging inside do_transform, even though the module already imports logging at the top. Please remove the inner import.

 def do_transform() -> None:
     """One run function to convert the dataset."""
-    import logging
-
     logger = logging.getLogger(__name__)

docker-compose.yaml (1)

54-55: Attach both services to the same networks

llm_training is only on mlflow-network while ollama stays on the default bridge, so name resolution for ollama still breaks exactly as noted earlier. Attach both containers to both networks so the training job can reach http://ollama:11434.

   ollama:
     ...
-    runtime: nvidia
+    runtime: nvidia
+    networks:
+      - default
+      - mlflow-network
   ...
   llm_training:
     ...
-    networks:
-      - mlflow-network
+    networks:
+      - default
+      - mlflow-network

run_pipeline.sh (1)

129-134: Fix the JSON payload syntax

The extra ')' after the jq block is still here, which causes bash to abort with “syntax error near unexpected token ')'”. Please drop that stray parenthesis.

 JSON_PAYLOAD=$(jq -n \
     --arg name "custom-model" \
     --arg blob_name "$BLOB_NAME" \
     --arg gguf_file "$GGUF_FILE" \
-    '{name: $name, files: {($gguf_file): $blob_name}}')')
+    '{name: $name, files: {($gguf_file): $blob_name}}')

testing_model/models.py (1)

281-284: Convert structured output schema before calling OpenAI.

response_format still receives the Pydantic class itself, and the async client will raise a serialization error on the first structured call. Please materialize the schema into the {"type": "json_schema", ...} payload that the API expects.

         if schema is not None:
-            kwargs["response_format"] = schema
+            if hasattr(schema, "model_json_schema"):
+                schema_dict = schema.model_json_schema()
+            elif hasattr(schema, "schema"):
+                schema_dict = schema.schema()
+            else:
+                raise TypeError("schema must expose model_json_schema() or schema()")
+            kwargs["response_format"] = {
+                "type": "json_schema",
+                "json_schema": {
+                    "name": schema.__name__,
+                    "schema": schema_dict,
+                    "strict": True,
+                },
+            }

training_model/dpo_train.py (1)

39-70: Resolve Hydra working-directory handling for DPO data files.

Hydra switches cwd to the run output directory, so Path.cwd() / cfg.paths.data_dir looks under outputs/... and both validation and preparation fail to find real datasets—exactly what we discussed earlier. Please derive the base path via hydra.utils.get_original_cwd() (with a safe fallback) in both validate_dpo_config and prepare_dpo_data.

-    data_dir = Path.cwd() / cfg.paths.data_dir
+    try:
+        from hydra.core.hydra_config import HydraConfig
+        from hydra.utils import get_original_cwd
+
+        if HydraConfig.initialized():
+            base_dir = Path(get_original_cwd())
+        else:
+            base_dir = Path.cwd()
+    except (ImportError, ValueError):
+        base_dir = Path.cwd()
+
+    data_dir = base_dir / cfg.paths.data_dir
@@
-    data_dir = Path.cwd() / cfg.paths.data_dir
+    try:
+        from hydra.core.hydra_config import HydraConfig
+        from hydra.utils import get_original_cwd
+
+        if HydraConfig.initialized():
+            base_dir = Path(get_original_cwd())
+        else:
+            base_dir = Path.cwd()
+    except (ImportError, ValueError):
+        base_dir = Path.cwd()
+
+    data_dir = base_dir / cfg.paths.data_dir

training_model/config_validation.py (1)

349-357: Read the WandB config from logging.wandb.

Validation still looks under cfg.wandb, so every legitimate config with logging_backend: wandb is rejected—mirroring the earlier feedback. Please inspect cfg.logging.wandb instead.

-        if hasattr(cfg, "wandb"):
-            wandb_cfg = cfg.wandb
-            if not hasattr(wandb_cfg, "project_name") or not wandb_cfg.project_name:
-                logger.error("Wandb project name is required")
-                return False
-        else:
-            logger.error("Wandb backend selected but no wandb configuration found")
-            return False
+        wandb_cfg = getattr(logging_cfg, "wandb", None)
+        if not wandb_cfg:
+            logger.error("Wandb backend selected but no logging.wandb configuration found")
+            return False
+        project_name = getattr(wandb_cfg, "project_name", None)
+        if not project_name:
+            logger.error("Wandb project name is required")
+            return False

training_model/grpo_train.py (2)

75-94: Guard against missing grpo / paths sections before validation.

This still dereferences cfg.grpo and cfg.paths unconditionally, so configurations that omit either section (which Hydra allows) now fail with AttributeError, exactly as called out in the earlier review. Please reintroduce the existence checks (or reuse the shared validator) before accessing their members and bail out with a clear log message when they’re absent.

---

758-760: Respect user-specified generation overrides.

generation_config is a plain dict, so getattr(..., "temperature", 1.0) always falls back to your literals and silently ignores GRPO overrides defined in the config—same regression reported previously. Swap these to .get(...) so custom settings actually flow into GRPOConfig.

-        temperature=getattr(generation_config, "temperature", 1.0),
-        top_p=getattr(generation_config, "top_p", 1.0),
-        top_k=getattr(generation_config, "top_k", 50),
+        temperature=generation_config.get("temperature", 1.0),
+        top_p=generation_config.get("top_p", 1.0),
+        top_k=generation_config.get("top_k", 50),

evaluation/deepeval_integration.py (1)

124-132: Fail fast when CustomOpenAIModel isn’t imported.

_ensure_models_imported() still leaves CustomOpenAIModel as None when the optional dependency is missing, and _get_custom_api_model() immediately calls it, triggering TypeError: 'NoneType' object is not callable—the same regression raised in the prior review. Please guard after the lazy import (and optionally invoke _load_environment_if_needed() so .env loading matches the other paths) before instantiating.

     _ensure_models_imported()
+    _load_environment_if_needed()
+    if CustomOpenAIModel is None:
+        raise RuntimeError(
+            "CustomOpenAIModel not available — install Deepeval custom API dependencies."
+        )

🧹 Nitpick comments (10)

training_model/exceptions.py (1)

7-8: Consider adding an __all__ export list.

Adding an explicit __all__ list would document the public API and improve IDE support.

Apply this diff to add the export list:

 """
 
 
+__all__ = [
+    "LLMLoRaError",
+    "ConfigurationError",
+    "ModelLoadingError",
+    "TrainingError",
+    "ConversionError",
+    "DataProcessingError",
+    "ExperimentTrackingError",
+    "ResourceManagementError",
+]
+
+
 class LLMLoRaError(Exception):

tools/dataset/find_threat_question_problems.py (1)

60-60: Move import to top of file.

Importing defaultdict mid-file violates PEP 8 conventions. All imports should be placed at the module's top.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path
 
 data_path = Path("T:/projects/LLM_LoRa/LLM_LoRa/data/dataset_ru_extended.json")

Then remove line 60:

 # Group by type
-from collections import defaultdict
-
 by_type = defaultdict(list)

tools/dataset/find_question_action_problems.py (2)

39-39: Move import to top of file.

Importing defaultdict mid-file violates PEP 8 conventions.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path

Then remove line 39:

 # Group by action type
-from collections import defaultdict
-
 by_action = defaultdict(list)

---

47-52: Remove unused loop variable.

The reason variable is unpacked in the loop but never used. Consider using _ for unused variables or remove if unnecessary.

Apply this diff:

 for action, items in sorted(by_action.items()):
     print(f"\n{action}: {len(items)} examples")
-    for i, (key, msg, reason) in enumerate(items[:5], 1):
+    for i, (key, msg, _) in enumerate(items[:5], 1):
         msg_short = msg[:70] + "..." if len(msg) > 70 else msg
         print(f"  {i}. {key}")
         print(f'     "{msg_short}"')

evaluation/game_evaluation.py (1)

42-44: Simplify by inlining the exception raise.

The _raise_action_mismatch() helper adds unnecessary indirection for a single raise statement. Consider inlining it directly at line 30.

Apply this diff:

         predicted_action = model_answer_dict["Content"]["Action"]
         correct_action = correct_answer_dict["Content"]["Action"]
         if predicted_action != correct_action:
-            _raise_action_mismatch()
+            raise AssertionError("Action doesn't match")
     except JSONDecodeError as err:
         error_msg = "Model answer or correct answer is not in JSON format"
         raise AssertionError(error_msg) from err

Then remove the helper function:

-
-
-def _raise_action_mismatch() -> None:
-    """Helper to raise a standardized AssertionError for action mismatch."""
-    raise AssertionError("Action doesn't match")

tools/dataset/find_future_tense_problems.py (1)

52-52: Move import to top of file.

Importing defaultdict mid-file violates PEP 8 conventions.

Apply this diff:

 import json
+from collections import defaultdict
 from pathlib import Path

Then remove line 52:

 # Group by action
-from collections import defaultdict
-
 by_action = defaultdict(list)

tools/dataset/analyze_dataset.py (1)

59-68: Remove unused loop variable.

The key variable is declared but never used in the loop body.

Apply this diff:

 # Incomplete examples
 incomplete = 0
-for key, example in examples.items():
+for example in examples.values():
     prompt_data = example.get("prompt", {})
     answer_data = example.get("answer", {})

training_model/__init__.py (1)

71-77: Improve numeric string detection.

The current logic for detecting numeric strings on line 76 may have edge cases. For example, multiple minus signs would pass the digit check after replacement.

Use explicit type conversion with exception handling:

         for key, raw_value in filtered_overrides.items():
             value = raw_value
-            # Convert string values to appropriate types
-            if isinstance(value, str) and value.lower() in ["true", "false"]:
-                value = value.lower() == "true"
-            elif isinstance(value, str) and value.replace(".", "").replace("-", "").isdigit():
-                value = float(value) if "." in value else int(value)
+            # Convert string values to appropriate types
+            if isinstance(value, str):
+                if value.lower() in ["true", "false"]:
+                    value = value.lower() == "true"
+                else:
+                    # Try numeric conversion
+                    try:
+                        value = int(value)
+                    except ValueError:
+                        try:
+                            value = float(value)
+                        except ValueError:
+                            pass  # Keep as string

tests/integration/test_training_functions.py (1)

58-62: Consider honoring tokenize parameter for completeness.

The mock apply_chat_template accepts but ignores the tokenize parameter. While current tests only use tokenize=False, honoring this parameter would make the mock more realistic.

def apply_chat_template(messages: list, tokenize: bool = False) -> str | list:
    parts = [f"{m['role'].upper()}: {m['content']}" for m in messages]
    text = "\n".join(parts)
    if tokenize:
        return tok.encode(text)
    return text

testing_model/__main__.py (1)

65-145: Log exceptions and mark intentionally unused arguments

Please switch the logger.error calls inside the wrappers to logger.exception so we keep stack traces, and prefix the intentionally unused correct_answer parameters with an underscore (while updating the call site in the first wrapper). This satisfies Ruff’s TRY400/ARG001 findings and makes the intent explicit.

     async def wrapper_unintended_answer_mention(
-        model_answer: str, correct_answer: str, **kwargs: dict
+        model_answer: str, _correct_answer: str, **kwargs: dict
     ) -> tuple[bool, float, str]:
         ...
-            return await test_unintended_answer_mention_async(
-                cfg, user_input, model_answer, correct_answer
-            )
+            return await test_unintended_answer_mention_async(
+                cfg, user_input, model_answer, _correct_answer
+            )
         except Exception as e:
-            logger.error(
+            logger.exception(
                 f"Failed to initialize evaluation model for unintended_answer_mention: {e}"
             )
             logger.error(
                 "Check your DeepEval config and API credentials (MISTRAL_API, CUSTOM_API_KEY)"
             )
             raise

     async def wrapper_russian_language_quality(
-        model_answer: str, correct_answer: str, **kwargs: dict
+        model_answer: str, _correct_answer: str, **_kwargs: dict
     ) -> tuple[bool, float, str]:
         ...
         except Exception as e:
-            logger.error(
+            logger.exception(
                 f"Failed to initialize evaluation model for russian_language_quality: {e}"
             )
             logger.error(
                 "Check your DeepEval config and API credentials (MISTRAL_API, CUSTOM_API_KEY)"
             )
             raise

     async def wrapper_game_context_appropriateness(
-        model_answer: str, correct_answer: str, **kwargs: dict
+        model_answer: str, _correct_answer: str, **kwargs: dict
     ) -> tuple[bool, float, str]:
         ...
         except Exception as e:
-            logger.error(
+            logger.exception(
                 f"Failed to initialize evaluation model for game_context_appropriateness: {e}"
             )
             logger.error(
                 "Check your DeepEval config and API credentials (MISTRAL_API, CUSTOM_API_KEY)"
             )
             raise

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df708b and c348a98.

⛔ Files ignored due to path filters (2)

• data/translate.csv is excluded by !**/*.csv
• poetry.lock is excluded by !**/*.lock

📒 Files selected for processing (73)

• .dvc/.gitignore (1 hunks)
• .dvc/config (1 hunks)
• .dvcignore (1 hunks)
• .flake8 (0 hunks)
• .gitignore (1 hunks)
• .pre-commit-config.yaml (1 hunks)
• Dockerfile (3 hunks)
• Dockerfile_cuda (1 hunks)
• Dockerfile_ollama (1 hunks)
• README.md (1 hunks)
• conf/config.yaml (1 hunks)
• data.dvc (1 hunks)
• data/daedalus_eng.json (0 hunks)
• data/test_eng.json (0 hunks)
• data/test_ru.json (0 hunks)
• data/values.txt (0 hunks)
• dev.py (1 hunks)
• docker-compose.yaml (1 hunks)
• docs/CONTRIBUTING.md (1 hunks)
• docs/README_RU.md (1 hunks)
• docs/api_reference.rst (1 hunks)
• docs/conf.py (2 hunks)
• docs/docker_deployment.rst (1 hunks)
• docs/evaluation.rst (1 hunks)
• docs/fire_cli.rst (1 hunks)
• docs/hydra_config.rst (7 hunks)
• docs/index.rst (1 hunks)
• docs/requirements.txt (1 hunks)
• docs/rkllm_conversion.rst (1 hunks)
• docs/testing_model.rst (0 hunks)
• docs/training_methods.rst (1 hunks)
• docs/training_model.rst (1 hunks)
• evaluation/__init__.py (1 hunks)
• evaluation/deepeval_integration.py (1 hunks)
• evaluation/game_evaluation.py (3 hunks)
• evaluation/model_evaluation.py (1 hunks)
• legacy/llama31_train.py (0 hunks)
• main.py (1 hunks)
• model.dvc (1 hunks)
• mypy.ini (0 hunks)
• pyproject.toml (2 hunks)
• requirements.txt (1 hunks)
• rkllm_files/Dockerfile.rkllm (1 hunks)
• rkllm_files/entrypoint.sh (1 hunks)
• rkllm_files/rkllm_converting.py (1 hunks)
• run_pipeline.sh (3 hunks)
• testing_model/__init__.py (1 hunks)
• testing_model/__main__.py (3 hunks)
• testing_model/deepeval_func.py (0 hunks)
• testing_model/models.py (1 hunks)
• testing_model/test.py (0 hunks)
• tests/README.md (1 hunks)
• tests/__init__.py (1 hunks)
• tests/integration/test_training_functions.py (1 hunks)
• tests/test_requirements.txt (1 hunks)
• tests/unit/test_dpo_grpo.py (1 hunks)
• tools/dataset/analyze_dataset.py (1 hunks)
• tools/dataset/analyze_game_coverage.py (1 hunks)
• tools/dataset/find_future_tense_problems.py (1 hunks)
• tools/dataset/find_history_ending_problems.py (1 hunks)
• tools/dataset/find_question_action_problems.py (1 hunks)
• tools/dataset/find_threat_question_problems.py (1 hunks)
• tools/dataset/validate_extended_dataset.py (1 hunks)
• training_model/__init__.py (1 hunks)
• training_model/__main__.py (3 hunks)
• training_model/config_validation.py (1 hunks)
• training_model/data_preparation.py (1 hunks)
• training_model/dpo_train.py (1 hunks)
• training_model/exceptions.py (1 hunks)
• training_model/grpo_train.py (5 hunks)
• training_model/logging_config.py (2 hunks)
• training_model/logging_utils.py (1 hunks)
• training_model/memory_utils.py (1 hunks)

💤 Files with no reviewable changes (10)

• data/daedalus_eng.json
• data/values.txt
• docs/testing_model.rst
• mypy.ini
• data/test_ru.json
• data/test_eng.json
• testing_model/deepeval_func.py
• .flake8
• testing_model/test.py
• legacy/llama31_train.py

🧰 Additional context used

🧬 Code graph analysis (17)

testing_model/__main__.py (5)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

evaluation/model_evaluation.py (2)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)

training_model/logging_config.py (1)

• configure_logging (73-119)

evaluation/deepeval_integration.py (7)

• test_game_context_appropriateness (304-355)
• test_game_context_appropriateness_async (436-484)
• test_russian_language_quality (279-301)
• test_russian_language_quality_async (400-433)
• test_unintended_answer_mention (235-276)
• test_unintended_answer_mention_async (358-397)
• _get_evaluation_model (134-162)

training_model/__init__.py (4)

• wrapper_unintended_answer_mention (321-327)
• wrapper_russian_language_quality (329-332)
• wrapper_game_context_appropriateness (334-341)
• test (253-370)

run_pipeline.sh (1)

training_model/__init__.py (1)

• pipeline (372-424)

training_model/__main__.py (4)

evaluation/model_evaluation.py (2)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)

training_model/logging_config.py (1)

• configure_logging (73-119)

training_model/one_file_train.py (1)

• main_train (1934-1976)

training_model/optuna.py (1)

• optuna_optimize (111-190)

training_model/__init__.py (7)

evaluation/model_evaluation.py (2)

• dataset_to_json_for_test (86-119)
• test_llm (904-969)

training_model/config_validation.py (2)

• get_validation_summary (434-468)
• validate_complete_config (393-431)

training_model/logging_config.py (1)

• configure_logging (73-119)

training_model/one_file_train.py (3)

• convert_to_gguf (1266-1331)
• convert_to_rkllm (1334-1511)
• train (934-1171)

training_model/optuna.py (1)

• optuna_optimize (111-190)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

evaluation/deepeval_integration.py (3)

• test_game_context_appropriateness (304-355)
• test_russian_language_quality (279-301)
• test_unintended_answer_mention (235-276)

testing_model/__init__.py (3)

evaluation/deepeval_integration.py (2)

• test_from_dataset (558-619)
• test_mention_number_of_values (194-232)

evaluation/model_evaluation.py (2)

• test_llm (904-969)
• test_via_llamacpp (1054-1144)

testing_model/models.py (3)

• CustomLocalModel (12-85)
• CustomMistralModel (88-198)
• CustomOpenAIModel (201-294)

tests/integration/test_training_functions.py (1)

training_model/one_file_train.py (8)

• change_dir (169-186)
• convert_to_gguf (1266-1331)
• data_preparation (447-559)
• generate_and_tokenize_prompt (244-280)
• generate_prompt (189-216)
• model_merge_for_converting (562-634)
• quantize_model (1680-1735)
• tokenize (219-241)

training_model/dpo_train.py (3)

training_model/logging_utils.py (3)

• get_report_to_backend (11-33)
• log_dataset_samples (263-350)
• mlflow_phase_run (37-78)

training_model/memory_utils.py (2)

• comprehensive_memory_cleanup (55-107)
• log_memory_usage (35-52)

training_model/optimizer_factory.py (2)

• create_optimizer (154-173)
• get_optimizer_config_updates (176-197)

training_model/config_validation.py (2)

training_model/dpo_train.py (1)

• validate_dpo_config (19-52)

training_model/grpo_train.py (1)

• validate_grpo_config (66-96)

training_model/logging_utils.py (1)

training_model/logging_config.py (1)

• log_dict (134-158)

training_model/logging_config.py (1)

rkllm_files/entrypoint.sh (1)

• log (8-10)

tests/unit/test_dpo_grpo.py (2)

training_model/dpo_train.py (3)

• dpo_train (141-305)
• prepare_dpo_data (55-138)
• validate_dpo_config (19-52)

training_model/grpo_train.py (8)

• grpo_train (556-863)
• debug_reward_function (99-146)
• prepare_grpo_data (444-553)
• reward_function (149-233)
• validate_grpo_config (66-96)
• reward_function_json_validation (236-289)
• reward_function_action_extraction (292-400)
• reward_function_combined (403-441)

training_model/data_preparation.py (1)

evaluation/model_evaluation.py (1)

• get_user_prompt (32-61)

dev.py (2)

evaluation/model_evaluation.py (1)

• run_tests (357-476)

training_model/logging_config.py (1)

• format (59-70)

training_model/grpo_train.py (4)

training_model/logging_utils.py (3)

• get_report_to_backend (11-33)
• log_dataset_samples (263-350)
• mlflow_phase_run (37-78)

training_model/memory_utils.py (2)

• comprehensive_memory_cleanup (55-107)
• log_memory_usage (35-52)

training_model/optimizer_factory.py (2)

• create_optimizer (154-173)
• get_optimizer_config_updates (176-197)

training_model/utils.py (1)

• get_generation_config (17-92)

main.py (1)

training_model/__init__.py (1)

• LLMLoRaCLI (32-546)

evaluation/deepeval_integration.py (2)

testing_model/models.py (3)

• CustomLocalModel (12-85)
• CustomMistralModel (88-198)
• CustomOpenAIModel (201-294)

training_model/utils.py (1)

• _load_environment_if_needed (95-103)

evaluation/model_evaluation.py (3)

evaluation/game_evaluation.py (1)

• test_actions (7-39)

training_model/data_preparation.py (1)

• get_user_prompt (12-23)

training_model/__init__.py (1)

• test (253-370)

🪛 Checkov (3.2.334)

rkllm_files/Dockerfile.rkllm

[high] 19-19: Ensure that certificate validation isn't disabled with the pip '--trusted-host' option

(CKV2_DOCKER_4)

🪛 GitHub Actions: Lint with pre-commit

.pre-commit-config.yaml

[error] 1-1: Command failed with exit code 1. pre-commit hook failures: end-of-file-fixer, trailing-whitespace, mixed-line-ending, ruff, ruff-format. Some files were modified by hooks; review changes.

🪛 LanguageTool

docs/README_RU.md

[uncategorized] ~173-~173: Перед знаками препинания не должно быть пробела: «конфигурация:»
Context: ...е файлы: - conf/config.yaml: Основная конфигурация - conf/model/: Настройки моделей - conf/training/: Г...

(Punct_and_spaces)

---

[uncategorized] ~174-~174: Перед знаками препинания не должно быть пробела: «моделей:»
Context: ...конфигурация - conf/model/: Настройки моделей - conf/training/: Гиперпараметры обучения ### Пример кон...

(Punct_and_spaces)

docs/CONTRIBUTING.md

[grammar] ~262-~262: Use a hyphen to join words.
Context: ...tion Style - Docstrings: Use Google style docstrings - Comments: Explain...

(QB_NEW_EN_HYPHEN)

---

[uncategorized] ~443-~443: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...king changes - MINOR: New features, backwards compatible - PATCH: Bug fixes, backwards compa...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🪛 Ruff (0.14.2)

testing_model/__main__.py

87-89: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

90-92: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

96-96: Unused function argument: correct_answer

(ARG001)

---

96-96: Unused function argument: kwargs

(ARG001)

---

108-110: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

111-113: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

117-117: Unused function argument: correct_answer

(ARG001)

---

139-141: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

142-144: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

tools/dataset/analyze_dataset.py

59-59: Loop control variable key not used within loop body

Rename unused key to _key

(B007)

training_model/__init__.py

5-5: Unused noqa directive (non-enabled: F401)

Remove unused noqa directive

(RUF100)

---

116-116: Avoid specifying long messages outside the exception class

(TRY003)

---

160-160: Avoid specifying long messages outside the exception class

(TRY003)

---

330-330: Unused function argument: correct_answer

(ARG001)

---

330-330: Unused function argument: kwargs

(ARG001)

---

335-335: Unused function argument: correct_answer

(ARG001)

---

405-405: Avoid specifying long messages outside the exception class

(TRY003)

tests/integration/test_training_functions.py

58-58: Unused function argument: tokenize

(ARG001)

training_model/dpo_train.py

165-165: Avoid specifying long messages outside the exception class

(TRY003)

---

193-193: Avoid specifying long messages outside the exception class

(TRY003)

evaluation/game_evaluation.py

7-7: Unused function argument: kwargs

(ARG001)

---

44-44: Avoid specifying long messages outside the exception class

(TRY003)

training_model/config_validation.py

169-169: Do not catch blind exception: Exception

(BLE001)

---

181-181: Redundant exception object included in logging.exception call

(TRY401)

---

424-424: Redundant exception object included in logging.exception call

(TRY401)

---

464-464: Do not catch blind exception: Exception

(BLE001)

tools/dataset/find_question_action_problems.py

47-47: Loop control variable reason not used within loop body

(B007)

training_model/logging_utils.py

70-71: Remove exception handler; error is immediately re-raised

(TRY203)

---

103-103: Do not catch blind exception: Exception

(BLE001)

---

144-144: Do not catch blind exception: Exception

(BLE001)

---

188-188: Do not catch blind exception: Exception

(BLE001)

---

222-222: Do not catch blind exception: Exception

(BLE001)

---

256-256: Consider moving this statement to an else block

(TRY300)

---

259-259: Redundant exception object included in logging.exception call

(TRY401)

---

349-349: Do not catch blind exception: Exception

(BLE001)

training_model/logging_config.py

93-93: Avoid specifying long messages outside the exception class

(TRY003)

---

97-97: Avoid specifying long messages outside the exception class

(TRY003)

tests/unit/test_dpo_grpo.py

632-632: Unused method argument: mock_dpo_config

(ARG002)

---

700-700: Unused method argument: mock_grpo_config

(ARG002)

dev.py

1-1: Shebang is present but file is not executable

(EXE001)

---

40-40: Do not catch blind exception: Exception

(BLE001)

---

97-97: subprocess call: check for execution of untrusted input

(S603)

---

339-339: Abstract raise to an inner function

(TRY301)

---

339-339: Avoid specifying long messages outside the exception class

(TRY003)

---

340-340: subprocess call: check for execution of untrusted input

(S603)

---

351-351: Abstract raise to an inner function

(TRY301)

---

351-351: Avoid specifying long messages outside the exception class

(TRY003)

---

353-353: subprocess call: check for execution of untrusted input

(S603)

---

377-377: Do not catch blind exception: Exception

(BLE001)

---

390-390: Do not catch blind exception: Exception

(BLE001)

---

396-396: Do not catch blind exception: Exception

(BLE001)

training_model/memory_utils.py

139-139: Do not catch blind exception: Exception

(BLE001)

training_model/grpo_train.py

226-229: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

236-236: Unused function argument: kwargs

(ARG001)

---

285-285: Do not catch blind exception: Exception

(BLE001)

---

286-286: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

396-396: Do not catch blind exception: Exception

(BLE001)

---

397-397: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

579-579: Avoid specifying long messages outside the exception class

(TRY003)

---

672-672: Avoid specifying long messages outside the exception class

(TRY003)

evaluation/deepeval_integration.py

30-30: Do not catch blind exception: Exception

(BLE001)

---

44-44: Do not catch blind exception: Exception

(BLE001)

---

71-71: Avoid specifying long messages outside the exception class

(TRY003)

---

95-95: Unused function argument: cfg

(ARG001)

---

99-99: Avoid specifying long messages outside the exception class

(TRY003)

---

162-162: Avoid specifying long messages outside the exception class

(TRY003)

---

184-184: subprocess call: check for execution of untrusted input

(S603)

---

219-219: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

236-236: Unused function argument: expected_answer

(ARG001)

---

263-263: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

359-359: Unused function argument: expected_answer

(ARG001)

---

383-383: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

---

541-541: Do not catch blind exception: Exception

(BLE001)

---

544-544: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

560-560: Unused function argument: test_dataset

(ARG001)

testing_model/models.py

25-25: Unused method argument: args

(ARG002)

---

26-26: Unused method argument: kwargs

(ARG002)

---

105-105: Unused method argument: args

(ARG002)

---

106-106: Unused method argument: kwargs

(ARG002)

---

219-219: Unused method argument: args

(ARG002)

---

220-220: Unused method argument: kwargs

(ARG002)

evaluation/model_evaluation.py

49-49: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

49-49: String contains ambiguous о (CYRILLIC SMALL LETTER O). Did you mean o (LATIN SMALL LETTER O)?

(RUF001)

---

49-49: String contains ambiguous р (CYRILLIC SMALL LETTER ER). Did you mean p (LATIN SMALL LETTER P)?

(RUF001)

---

54-54: String contains ambiguous Т (CYRILLIC CAPITAL LETTER TE). Did you mean T (LATIN CAPITAL LETTER T)?

(RUF001)

---

58-58: String contains ambiguous О (CYRILLIC CAPITAL LETTER O). Did you mean O (LATIN CAPITAL LETTER O)?

(RUF001)

---

58-58: String contains ambiguous е (CYRILLIC SMALL LETTER IE). Did you mean e (LATIN SMALL LETTER E)?

(RUF001)

---

275-275: Do not catch blind exception: Exception

(BLE001)

---

558-558: Redundant exception object included in logging.exception call

(TRY401)

---

612-612: Unused noqa directive (non-enabled: SIM113)

Remove unused noqa directive

(RUF100)

---

618-618: Redundant exception object included in logging.exception call

(TRY401)

---

699-699: Abstract raise to an inner function

(TRY301)

---

711-711: Do not catch blind exception: Exception

(BLE001)

---

734-734: Unpacked variable score is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

---

738-738: Abstract raise to an inner function

(TRY301)

---

751-751: Do not catch blind exception: Exception

(BLE001)

---

773-773: Redundant exception object included in logging.exception call

(TRY401)

---

828-828: Unused noqa directive (non-enabled: SIM113)

Remove unused noqa directive

(RUF100)

---

963-963: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

964-964: Redundant exception object included in logging.exception call

(TRY401)

---

985-985: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

992-992: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1066-1066: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1071-1071: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF002)

---

1114-1114: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)

---

1115-1115: String contains ambiguous У (CYRILLIC CAPITAL LETTER U). Did you mean Y (LATIN CAPITAL LETTER Y)?

(RUF001)

---

1116-1116: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

1117-1117: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

rkllm_files/rkllm_converting.py

1-1: Shebang is present but file is not executable

(EXE001)

---

114-114: Avoid specifying long messages outside the exception class

(TRY003)

---

190-190: Consider moving this statement to an else block

(TRY300)

---

193-193: Redundant exception object included in logging.exception call

(TRY401)

🪛 Shellcheck (0.11.0)

run_pipeline.sh

[warning] 76-76: Quote this to prevent word splitting.

(SC2046)

---

[warning] 77-77: Quote this to prevent word splitting.

(SC2046)

---

[error] 124-124: Parsing stopped here. Is this keyword correctly matched up?

(SC1089)

🔇 Additional comments (24)

training_model/exceptions.py (1)

1-95: Well-structured exception hierarchy!

The exception classes are well-organized with a clear inheritance hierarchy and comprehensive docstrings. Each exception type has specific, documented use cases that will aid debugging and error handling throughout the framework.

model.dvc (1)

1-6: LGTM! Valid DVC model tracking configuration.

The DVC metadata correctly tracks the model directory with appropriate hash, size, and file count information.

tests/__init__.py (1)

1-2: LGTM! Standard package marker.

This correctly makes the tests directory a Python package.

.dvc/.gitignore (1)

1-3: LGTM! Standard DVC ignore patterns.

These patterns correctly exclude local DVC configuration and cache directories from version control.

.dvcignore (1)

1-3: LGTM! Valid initial .dvcignore setup.

The file is correctly structured with documentation reference. Patterns can be added later as needed.

data.dvc (1)

1-6: LGTM! Valid DVC data tracking configuration.

The DVC metadata correctly tracks the data directory with appropriate hash, size, and file count information.

.dvc/config (2)

1-3: LGTM! Appropriate DVC cache settings.

The hardlink cache type efficiently manages disk space, and protected mode prevents accidental cache corruption.

---

4-7: Document DVC remote configuration and setup requirements.

The DVC remote configuration uses an SMB/UNC path (//diskstation_105/dvc_storage) but lacks documentation. While no active DVC commands are found in the codebase, this should still be documented to prevent issues if the remote is used in the future or if team members need to set up the project.

Recommendations:

• Add DVC setup instructions to README or a dedicated setup guide, including SMB/CIFS requirements for different OS
• Clarify whether the NAS is required for all team members or only specific workflows
• Document fallback options for environments without NAS access (e.g., local storage or cloud alternatives)

.gitignore (1)

195-203: No issues found. The ignored patterns are correctly designated for personal/temporary files.

Verification confirms these files don't exist in the repository, were never committed, and are appropriately excluded from version control. These are personal/temporary files (configuration, notes, and local instructions) that the developer correctly prevents from being tracked.

README.md (1)

1-596: Excellent documentation improvements!

The README has been significantly enhanced with comprehensive sections covering installation, configuration, usage examples, troubleshooting, and more. The structure is clear, well-organized, and provides valuable guidance for users at all levels.

tests/test_requirements.txt (1)

1-22: LGTM! Well-structured test requirements file.

The test requirements file is well-organized with clear comments, appropriate minimum version specifications, and good separation of concerns between core testing frameworks and additional utilities.

docs/index.rst (1)

1-109: LGTM! Comprehensive documentation structure.

The documentation homepage is well-organized with clear sections covering features, supported models, quick start, architecture, and navigation to detailed documentation. The structure effectively guides users through the framework's capabilities.

Dockerfile_cuda (1)

26-30: LGTM! Proper CUDA-enabled llama.cpp build.

The llama.cpp build configuration correctly handles CUDA support with appropriate workarounds for the libcuda.so stub, uses Release mode, and leverages parallel compilation for efficiency.

rkllm_files/entrypoint.sh (1)

1-51: Well-structured entrypoint script.

The script properly handles container lifecycle with appropriate error management (set -e, error function), includes logging with timestamps, and correctly routes the convert command to the Python backend. The idle mode using tail -f /dev/null keeps the container running when no command is provided.

One optional enhancement: consider adding a trap to clean up resources on exit:

cleanup() {
    log "Container shutting down..."
    # Add cleanup logic here if needed
}
trap cleanup EXIT INT TERM

This ensures graceful shutdown handling, though the current implementation is functional for the intended use case.

docs/docker_deployment.rst (1)

1-50: Comprehensive Docker deployment documentation.

The documentation provides clear guidance on containerized deployment with well-organized sections covering base images, Docker Compose setup, Ollama integration, and production configurations. Examples are concrete and follow Docker best practices (non-root users, health checks, security considerations).

Verify that all referenced service names and paths match your actual repository structure:

• Service names in docker-compose examples (e.g., llmlora:train, ollama, training, inference)
• Dockerfile paths (e.g., Dockerfile.train, Dockerfile.ollama)
• Volume mount paths and configurations

docs/rkllm_conversion.rst (1)

1-50: Well-structured RKLLM conversion documentation.

The documentation comprehensively covers the conversion workflow with clear platform support tables, performance characteristics, and step-by-step processes. The progression from prerequisites through advanced deployment scenarios is logical and actionable.

Verify that the RKLLM toolkit API and imports referenced in the code examples (lines 222-254) match your rkllm-toolkit version:

• Import statement: from rkllm import RKLLM
• Methods: load_onnx(), build(), export_rkllm()
• Configuration parameters and return codes

These details are critical for users following the step-by-step conversion guide.

docs/training_model.rst (1)

1-110: Clear training module documentation overview.

The documentation appropriately structures the training_model package documentation with clear descriptions of supported training methods (SFT, DPO, GRPO) and a logical workflow progression. The submodule organization with automodule directives follows Sphinx best practices.

Verify that all referenced submodules and their public members are properly exported:

• Ensure each module listed (one_file_train, dpo_train, grpo_train, optuna, auth_utils, data_preparation, logging_config) exposes the documented __all__ list or uses module-level documentation
• Confirm that automodule directives will generate correctly by checking module imports and public API visibility

docs/README_RU.md (1)

1-100: Comprehensive Russian README documentation.

The Russian README provides thorough project documentation with clear sections covering setup, features, configuration, usage, and troubleshooting. The content is well-organized and mirrors the structure of the English version while maintaining idiomatic Russian.

Note on static analysis hints: LanguageTool flagged punctuation spacing at lines 173-174 regarding colons in the "Ключевые конфигурационные файлы:" section. These appear to be false positives, as the spacing is correct for the bullet-list context used in your configuration example. The formatting is appropriate.

docs/CONTRIBUTING.md (1)

260-287: Comprehensive contributing guidelines.

The document provides clear, actionable guidance for contributors with well-organized sections covering development setup, testing, documentation standards, and the PR process. Concrete commands and templates make it easy for contributors to follow the workflow.

Two minor grammar corrections suggested by static analysis (should use hyphens for compound adjectives):

• Line 262: Change "Use Google style docstrings" to "Use Google-style docstrings"
• Line 443: Change "backwards compatible" to "backwards-compatible"

These are minor nitpicks but align with English grammar best practices for compound adjectives.

requirements.txt (1)

1-50: Updated dependency manifest with CUDA 12.4 support.

The requirements have been refreshed to use the cu124 PyTorch wheel index and updated package versions across the ML/AI stack. The dependency set comprehensively covers training, evaluation, serving, and monitoring needs with appropriate version constraints.

Verify CUDA 12.4 (cu124) compatibility for key packages, especially:

• torch==2.6.0+cu124 and torchvision==0.21.0+cu124: Confirm these versions are stable and compatible with your target GPUs
• bitsandbytes==0.46.1: Verify it supports CUDA 12.4 (bitsandbytes has historically had CUDA compatibility issues)
• transformers==4.55.0: Ensure this recent version is stable and compatible with PyTorch 2.6.0

You can verify these by:

pip install -r requirements.txt --dry-run
# or test in a clean environment

training_model/__main__.py (2)

37-48: Verify path resolution robustness.

The data directory resolution uses parents[1] which assumes training_model/__main__.py is exactly two levels deep from the project root. This works currently but is fragile if the file moves or the project structure changes.

Consider verifying this assumption holds across deployment scenarios or making it more robust by searching upward for a marker file (e.g., pyproject.toml, .git):

def find_project_root(start_path: Path) -> Path:
    """Find project root by looking for marker files."""
    current = start_path.resolve()
    for parent in [current] + list(current.parents):
        if (parent / "pyproject.toml").exists() or (parent / ".git").exists():
            return parent
    return current  # fallback to current directory

---

49-52: LGTM!

The conditional Optuna optimization flow is well-structured, cleanly branching between optimization and standard training modes.

testing_model/__init__.py (1)

1-16: LGTM!

The public API reorganization cleanly exposes the new model wrappers and test utilities. Imports are well-structured and the __all__ list properly documents the public surface.

training_model/grpo_train.py (1)

47-54: isinstance with a union literal will crash immediately.

isinstance(param, Params4bit | Int8Params) raises TypeError: isinstance() argument 2 cannot be a union on Python 3.10+, so the conversion loop blows up before touching any parameters. Please switch to a tuple so execution reaches the intended skip path.

-        if isinstance(param, Params4bit | Int8Params):
+        if isinstance(param, (Params4bit, Int8Params)):
             continue

Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Docker-in-Docker pattern requires security consideration.

Installing Docker CLI allows the training container to control the host Docker daemon (typically via mounted /var/run/docker.sock). This grants the container significant privileges and should be carefully controlled.

Ensure that:

1. The socket mount is only enabled when RKLLM conversion is needed
2. Access is restricted in production environments
3. Alternative approaches (e.g., separate conversion service) have been considered

If docker-compose.yaml mounts the socket, verify it's conditionally enabled:

---

🏁 Script executed:

#!/bin/bash
# Check if docker-compose.yaml mounts docker.sock
rg -n "docker.sock" docker-compose.yaml docker-compose.yml

Length of output: 131

---

🏁 Script executed:

#!/bin/bash
# Examine docker-compose.yaml around line 47 to understand context
sed -n '35,55p' docker-compose.yaml

Length of output: 456

---

🏁 Script executed:

#!/bin/bash
# View entire docker-compose.yaml to understand service structure
cat docker-compose.yaml

Length of output: 1337

---

🏁 Script executed:

#!/bin/bash
# Check if Dockerfile contains any docker commands/usage
rg -n "docker" Dockerfile | head -20

Length of output: 420

---

Docker socket mount is unconditional and should be restricted to converter operations only.

The socket mount on llm_training service (docker-compose.yaml line 47) is enabled unconditionally with full read-write access, despite the Dockerfile comment stating it's only needed for RKLLM converter. The converter runs as a separate gated service (profiles: ["converter"]) but lacks the socket mount, creating an inconsistent and over-privileged setup.

Fix required:

• Either mount the socket only on rkllm_converter service (with conditional profile), OR
• Conditionally enable the mount on llm_training via profiles/environment variable when conversion is needed

This ensures the host Docker daemon is only accessible when necessary.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Poetry lock failure silently ignored.

Line 51 uses || true to ignore poetry lock failures. This may lead to inconsistent dependencies across container builds, as lock file generation errors won't be detected.

Consider handling lock failures more explicitly:

# Try to use existing lock, but allow regeneration if corrupted
RUN pip install poetry && \
    pip install pyyaml && \
    (poetry lock --check || poetry lock --no-interaction --no-ansi) && \
    poetry install --no-root --only main --no-interaction --no-ansi

🤖 Prompt for AI Agents

Dockerfile around lines 48 to 52: the Dockerfile currently silences poetry lock
failures with "|| true", which can hide dependency lock issues and produce
inconsistent builds; replace the silent ignore with an explicit check/regenerate
flow so the build either validates the existing lock or regenerates it, e.g.,
run poetry lock --check and if that fails run poetry lock --no-interaction
--no-ansi, then run poetry install, and do not swallow errors so the build fails
on unexpected lock errors.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify .exe extension requirement on Linux.

The symlink creates llama-quantize.exe in a Linux container, which is unusual. The comment mentions matching Windows naming convention, suggesting the calling code expects this exact filename.

Verify whether training_model code hardcodes the .exe extension or if this can be made platform-aware:

---

🏁 Script executed:

#!/bin/bash
# Check for hardcoded .exe references in training code
rg -n "llama-quantize\.exe" training_model/

Length of output: 218

---

I need to examine the broader context around these hardcoded references to see if there's platform detection logic or if the parameter can be overridden:

</parameter>
</invoke>

---

🏁 Script executed:

```shell
#!/bin/bash
# Check context around the hardcoded .exe references
rg -n -B5 -A5 "llama-quantize\.exe" training_model/one_file_train.py

Length of output: 924

---

🏁 Script executed:

#!/bin/bash
# Check function signature and how quantized_path is used
ast-grep --pattern $'def $_($$$quantized_path$$$) {
  $$$
}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Search for platform detection or conditional logic related to quantize
rg -n -i "platform|windows|linux|os\." training_model/one_file_train.py | head -20

Length of output: 1039

---

I need to check how this function is called to see if the parameter is being overridden or if it relies on the default:

</parameter>
</invoke>

---

🏁 Script executed:

```shell
#!/bin/bash
# Find all calls to quantize_model function
rg -n "quantize_model\(" training_model/ --type=py

Length of output: 138

---

🏁 Script executed:

#!/bin/bash
# Check if there are any tests or other references to the quantized_path parameter
rg -n "quantized_path\s*=" training_model/ --type=py

Length of output: 102

---

🏁 Script executed:

#!/bin/bash
# Look for platform detection at top level or imports
head -50 training_model/one_file_train.py | grep -n "import\|platform\|sys"

Length of output: 906

---

I need to check what the config actually contains for the quantized_path value:

</parameter>
</invoke>

---

🏁 Script executed:

```shell
#!/bin/bash
# Find config structure definition
rg -n "quantized_path" training_model/ --type=py -B2 -A2

Length of output: 1646

---

🏁 Script executed:

#!/bin/bash
# Look for config file or config defaults
fd -type f -name "*.yaml" -o -name "*.yml" -o -name "*config*" training_model/ 2>/dev/null | head -10

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Check context around line 1836-1841 to see full function call
sed -n '1830,1845p' training_model/one_file_train.py

Length of output: 761

---

Let me search for the config definition at the project level:

</parameter>
</invoke>

---

🏁 Script executed:

```shell
#!/bin/bash
# Search for config.yaml or similar at project root
find . -maxdepth 2 -type f \( -name "*.yaml" -o -name "*.yml" -o -name "config.py" \) 2>/dev/null | head -20

Length of output: 105

---

🏁 Script executed:

#!/bin/bash
# Search for where cfg.paths.quantized_path is defined
rg -n "quantized_path.*=" . --type=py --type=yaml | grep -v "^training_model"

Length of output: 383

---

🏁 Script executed:

#!/bin/bash
# Look in parent dirs and common config locations
rg -n "paths:" . --type=yaml -A5 | head -30

Length of output: 313

---

</parameter>
</invoke>

---

🏁 Script executed:

```shell
#!/bin/bash
# Read the full paths section from config.yaml
sed -n '163,180p' ./conf/config.yaml

Length of output: 604

---

🏁 Script executed:

#!/bin/bash
# Look for quantized_path in entire config.yaml
cat ./conf/config.yaml | grep -n "quantized_path" -B2 -A2

Length of output: 268

---

Fix hardcoded .exe extension in error message and config assumptions.

The .exe extension is hardcoded in two places with design issues:

1. conf/config.yaml (line 175): Config sets quantized_path: "llama-quantize.exe", but the commented alternative on line 174 shows platform variations are expected. Tests (test_training_functions.py lines 302, 327) use the platform-neutral "llama-quantize" without .exe, creating inconsistency.

2. training_model/one_file_train.py (line 1709): Error message hardcodes "llama-quantize.exe" as a string literal, but the actual executable path comes from the quantized_path parameter. If the config ever changes, the error message will be misleading.

The Dockerfile symlink on line 58 is a workaround for these hardcoded assumptions. Either make the naming truly platform-aware throughout or document that .exe is required specifically for the Docker environment.

🤖 Prompt for AI Agents

Dockerfile lines 57-58: the symlink creation is compensating for a hardcoded
".exe" assumption elsewhere; remove the workaround by making the executable name
platform-neutral across the codebase: set conf/config.yaml quantized_path to
"llama-quantize" (or derive it at runtime based on platform), update
training_model/one_file_train.py to reference the quantized_path parameter in
its error message instead of the literal "llama-quantize.exe", and update tests
to use the platform-neutral name; alternatively, if Docker must provide a
Windows-named binary, change the Dockerfile to create both names (llama-quantize
and llama-quantize.exe) or document the requirement—pick one approach and make
the changes consistently.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix typo: "ROGUE" should be "ROUGE"

The metric name is misspelled. It should be "ROUGE" (Recall-Oriented Understudy for Gisting Evaluation), not "ROGUE".

Apply this diff:

-* **Evaluation Metrics**: Support for BLEU, ROGUE, perplexity, and custom domain-specific metrics
+* **Evaluation Metrics**: Support for BLEU, ROUGE, perplexity, and custom domain-specific metrics

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	* **Evaluation Metrics**: Support for BLEU, ROGUE, perplexity, and custom domain-specific metrics
	* **Evaluation Metrics**: Support for BLEU, ROUGE, perplexity, and custom domain-specific metrics

🤖 Prompt for AI Agents

In docs/evaluation.rst around line 15, the metric name "ROGUE" is misspelled;
replace "ROGUE" with the correct "ROUGE" in the bullet text so the line reads "*
**Evaluation Metrics**: Support for BLEU, ROUGE, perplexity, and custom
domain-specific metrics".

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Return a JSON string from ollama_generate.

ollama_generate currently returns the raw response["response"], which is a dict when using structured outputs. Downstream checks (e.g., test_actions) call json.loads(model_answer), so passing a dict triggers a TypeError and every Ollama-backed test fails. Serialize the structured response to a string (or otherwise normalize both code paths to return the same type). Example fix:

-    logger.debug(response)
-    return response["response"]
+    logger.debug(response)
+    parsed = response["response"]
+    if isinstance(parsed, (dict, list)):
+        return json.dumps(parsed, ensure_ascii=False)
+    return str(parsed)

🤖 Prompt for AI Agents

In evaluation/model_evaluation.py around lines 140 to 148, ollama_generate
returns response["response"] which is a dict for structured outputs causing
downstream json.loads() callers to fail; ensure the function always returns a
JSON string by detecting if response["response"] is not a string and serializing
it (e.g., json.dumps(response["response"])) before returning, and add an import
json at the top if missing so both structured and plain-text paths return the
same type.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Guard against empty dataset before computing percentages

If the dataset loads but contains zero examples, the division on len(examples) raises ZeroDivisionError, terminating the validation run instead of reporting a failed check. Add an explicit empty-data guard before computing percentages.

-print(f"1. Dataset size: {len(examples)} examples")
+total_examples = len(examples)
+print(f"1. Dataset size: {total_examples} examples")
 print("   Target: ~300-330 examples")
-print(f"   Status: {'PASS' if 250 <= len(examples) <= 350 else 'FAIL'}")
+if total_examples == 0:
+    print("   Status: FAIL (dataset has no examples)")
+    sys.exit(1)
+print(f"   Status: {'PASS' if 250 <= total_examples <= 350 else 'FAIL'}")
@@
-razgovor_pct = (action_counts.get("Разговор", 0) / len(examples)) * 100
+razgovor_pct = (action_counts.get("Разговор", 0) / total_examples) * 100

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In tools/dataset/validate_extended_dataset.py around lines 31 to 40, the script
computes percentages using len(examples) without checking for an empty dataset
which can raise ZeroDivisionError; add an explicit guard that detects
len(examples) == 0 and handles it (log or print a clear "FAIL" status for size
and skip or set percentages to 0/NA) before any division, then proceed to
compute razgovor_pct only when examples > 0 so the validation run completes
gracefully for empty datasets.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Data directory resolution inconsistent with main.py.

This uses Path.cwd() / cfg.paths.data_dir which assumes the current working directory is the project root. However, training_model/__main__.py (lines 37-48) implements more robust absolute/relative path handling.

Consider extracting the path resolution logic into a shared helper:

def resolve_data_dir(cfg: DictConfig) -> Path:
    """Resolve data directory from config, handling absolute and relative paths."""
    data_dir_config = Path(cfg.paths.data_dir)
    if data_dir_config.is_absolute():
        return data_dir_config
    # Resolve relative to project root
    # Could use marker files like pyproject.toml instead of hardcoded parents
    return Path.cwd() / data_dir_config

Then use it consistently in both __main__.py and the CLI methods.

🤖 Prompt for AI Agents

In training_model/__init__.py around lines 118 to 120, the data directory is
resolved with Path.cwd() / cfg.paths.data_dir which is inconsistent with the
more robust handling in training_model/__main__.py; extract the resolution logic
into a shared helper (e.g. resolve_data_dir(cfg) that accepts the config, treats
absolute paths as-is and resolves relative paths against the project root or a
chosen marker file), replace the direct Path.cwd() usage with a call to that
helper, and update __main__.py and any CLI entrypoints to use the same helper so
path resolution is consistent across the package.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use Hydra’s original cwd when validating DPO/GRPO paths.

These validators still rely on Path.cwd(), so under Hydra they scan the transient output directory and falsely report missing datasets. Reuse the same get_original_cwd() fallback logic you already added in validate_paths_config for both DPO and GRPO checks.

-    work_dir = Path.cwd()
-    data_dir = Path(work_dir) / cfg.paths.data_dir
+    try:
+        from hydra.core.hydra_config import HydraConfig
+        from hydra.utils import get_original_cwd
+
+        if HydraConfig.initialized():
+            base_dir = Path(get_original_cwd())
+        else:
+            base_dir = Path.cwd()
+    except (ImportError, ValueError):
+        base_dir = Path.cwd()
+
+    data_dir = base_dir / cfg.paths.data_dir
@@
-    work_dir = Path.cwd()
-    data_dir = Path(work_dir) / cfg.paths.data_dir
+    try:
+        from hydra.core.hydra_config import HydraConfig
+        from hydra.utils import get_original_cwd
+
+        if HydraConfig.initialized():
+            base_dir = Path(get_original_cwd())
+        else:
+            base_dir = Path.cwd()
+    except (ImportError, ValueError):
+        base_dir = Path.cwd()
+
+    data_dir = base_dir / cfg.paths.data_dir

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	work_dir = Path.cwd()
	data_dir = Path(work_dir) / cfg.paths.data_dir
	train_file = data_dir / cfg.dpo.train_data
	val_file = data_dir / cfg.dpo.val_data
	if not train_file.exists():
	logger.error(f"DPO training data file not found: {train_file}")
	return False
	if not val_file.exists():
	logger.error(f"DPO validation data file not found: {val_file}")
	return False
	beta = getattr(cfg.dpo, "beta", None)
	if beta is not None and beta <= 0:
	logger.error("DPO beta parameter must be positive")
	return False
	max_length = getattr(cfg.dpo, "max_length", None)
	if max_length is not None and max_length <= 0:
	logger.error("DPO max_length must be positive")
	return False
	logger.info("DPO configuration validation passed")
	return True
	def validate_grpo_config(cfg: DictConfig) -> bool:
	"""Validate GRPO configuration parameters.
	Args:
	cfg: Configuration object
	Returns:
	bool: True if configuration is valid
	"""
	logger = logging.getLogger(__name__)
	if not hasattr(cfg, "grpo"):
	logger.info("GRPO configuration not found - skipping GRPO validation")
	return True
	if not hasattr(cfg, "paths") or cfg.paths is None:
	logger.error("Missing required GRPO parameters or cfg.paths: [paths section missing]")
	return False
	# Check GRPO-specific parameters in cfg.grpo
	grpo_params = ["num_generations", "val_data", "train_data"]
	missing_grpo_params = [p for p in grpo_params if not hasattr(cfg.grpo, p)]
	# Check path-based parameters in cfg.paths
	path_params = ["data_dir"]
	missing_path_params = [p for p in path_params if getattr(cfg.paths, p, None) is None]
	all_missing_params = []
	if missing_grpo_params:
	all_missing_params.extend([f"grpo.{p}" for p in missing_grpo_params])
	if missing_path_params:
	all_missing_params.extend([f"paths.{p}" for p in missing_path_params])
	if all_missing_params:
	logger.error(f"Missing required GRPO parameters or cfg.paths: {all_missing_params}")
	return False
	work_dir = Path.cwd()
	data_dir = Path(work_dir) / cfg.paths.data_dir
	train_file = data_dir / cfg.grpo.train_data
	val_file = data_dir / cfg.grpo.val_data
	if not train_file.exists():
	logger.error(f"GRPO training data file not found: {train_file}")
	return False
	if not val_file.exists():
	logger.error(f"GRPO validation data file not found: {val_file}")
	return False
	num_generations = getattr(cfg.grpo, "num_generations", None)
	if num_generations is not None and num_generations <= 0:
	logger.error("GRPO num_generations must be positive")
	return False
	epsilon = getattr(cfg.grpo, "epsilon", None)
	if epsilon is not None and (epsilon <= 0 or epsilon > 1):
	logger.error("GRPO epsilon must be between 0 and 1")
	return False
	temperature = getattr(cfg.grpo, "temperature", None)
	if temperature is not None and temperature <= 0:
	logger.error("GRPO temperature must be positive")
	try:
	from hydra.core.hydra_config import HydraConfig
	from hydra.utils import get_original_cwd
	if HydraConfig.initialized():
	base_dir = Path(get_original_cwd())
	else:
	base_dir = Path.cwd()
	except (ImportError, ValueError):
	base_dir = Path.cwd()
	data_dir = base_dir / cfg.paths.data_dir
	train_file = data_dir / cfg.dpo.train_data
	val_file = data_dir / cfg.dpo.val_data
	if not train_file.exists():
	logger.error(f"DPO training data file not found: {train_file}")
	return False
	if not val_file.exists():
	logger.error(f"DPO validation data file not found: {val_file}")
	return False
	beta = getattr(cfg.dpo, "beta", None)
	if beta is not None and beta <= 0:
	logger.error("DPO beta parameter must be positive")
	return False
	max_length = getattr(cfg.dpo, "max_length", None)
	if max_length is not None and max_length <= 0:
	logger.error("DPO max_length must be positive")
	return False
	logger.info("DPO configuration validation passed")
	return True
	def validate_grpo_config(cfg: DictConfig) -> bool:
	"""Validate GRPO configuration parameters.
	Args:
	cfg: Configuration object
	Returns:
	bool: True if configuration is valid
	"""
	logger = logging.getLogger(__name__)
	if not hasattr(cfg, "grpo"):
	logger.info("GRPO configuration not found - skipping GRPO validation")
	return True
	if not hasattr(cfg, "paths") or cfg.paths is None:
	logger.error("Missing required GRPO parameters or cfg.paths: [paths section missing]")
	return False
	# Check GRPO-specific parameters in cfg.grpo
	grpo_params = ["num_generations", "val_data", "train_data"]
	missing_grpo_params = [p for p in grpo_params if not hasattr(cfg.grpo, p)]
	# Check path-based parameters in cfg.paths
	path_params = ["data_dir"]
	missing_path_params = [p for p in path_params if getattr(cfg.paths, p, None) is None]
	all_missing_params = []
	if missing_grpo_params:
	all_missing_params.extend([f"grpo.{p}" for p in missing_grpo_params])
	if missing_path_params:
	all_missing_params.extend([f"paths.{p}" for p in missing_path_params])
	if all_missing_params:
	logger.error(f"Missing required GRPO parameters or cfg.paths: {all_missing_params}")
	return False
	try:
	from hydra.core.hydra_config import HydraConfig
	from hydra.utils import get_original_cwd
	if HydraConfig.initialized():
	base_dir = Path(get_original_cwd())
	else:
	base_dir = Path.cwd()
	except (ImportError, ValueError):
	base_dir = Path.cwd()
	data_dir = base_dir / cfg.paths.data_dir
	train_file = data_dir / cfg.grpo.train_data
	val_file = data_dir / cfg.grpo.val_data
	if not train_file.exists():
	logger.error(f"GRPO training data file not found: {train_file}")
	return False
	if not val_file.exists():
	logger.error(f"GRPO validation data file not found: {val_file}")
	return False
	num_generations = getattr(cfg.grpo, "num_generations", None)
	if num_generations is not None and num_generations <= 0:
	logger.error("GRPO num_generations must be positive")
	return False
	epsilon = getattr(cfg.grpo, "epsilon", None)
	if epsilon is not None and (epsilon <= 0 or epsilon > 1):
	logger.error("GRPO epsilon must be between 0 and 1")
	return False
	temperature = getattr(cfg.grpo, "temperature", None)
	if temperature is not None and temperature <= 0:
	logger.error("GRPO temperature must be positive")

🤖 Prompt for AI Agents

In training_model/config_validation.py around lines 215 to 304, the DPO/GRPO
validators use Path.cwd() which under Hydra points to the transient output dir
and causes false missing-file errors; change the work_dir calculation to reuse
the same get_original_cwd() fallback logic used in validate_paths_config (i.e.,
call get_original_cwd() if available and fall back to Path.cwd()), update both
the DPO and GRPO blocks to compute data_dir from that work_dir, and ensure
get_original_cwd is imported or accessible in this file.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Guard dataset logging when no logging block is provided.

Accessing cfg.logging directly raises an AttributeError for minimal configs, which crashes training the first time log_dataset_samples runs. Please guard the lookup so the helper becomes a no-op when the logging section is absent.

-        dataset_logging_cfg = getattr(cfg.logging, "dataset_logging", {})
+        logging_cfg = getattr(cfg, "logging", None)
+        dataset_logging_cfg = (
+            getattr(logging_cfg, "dataset_logging", {}) if logging_cfg else {}
+        )

🤖 Prompt for AI Agents

In training_model/logging_utils.py around lines 279 to 285, the code accesses
cfg.logging directly which raises AttributeError when the config omits the
logging block; change the guard to first check for the logging section (e.g.,
use getattr(cfg, "logging", None) or getattr(cfg, "logging", {}) ) and if it's
missing or falsy return early so the function becomes a no-op when there is no
logging section; then proceed to read dataset_logging via getattr on that safe
value and continue as before.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@testing_model/vllm_test.py`:
- Around line 9-17: The SYSTEM_PROMPT defines the contract asking the model to
return JSON with an 'Actions' key, but the evaluator code at line 65 reads from
'Content.Action' (singular), causing a mismatch that leads to KeyError or
scoring failures when the model follows the prompt. Align the key name between
the SYSTEM_PROMPT (lines 9-17) and the evaluator code at line 65 so they both
reference the same key, and add guard/error handling around the JSON key access
to safely handle cases where the expected keys are missing or misnamed.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f42fb8d-be2b-4d56-826e-e97ec3add46a

📥 Commits

Reviewing files that changed from the base of the PR and between c348a98 and b3cdc73.

📒 Files selected for processing (4)

• .gitignore
• .readthedocs.yaml
• conf/config.yaml
• testing_model/vllm_test.py

✅ Files skipped from review due to trivial changes (1)

• .gitignore

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align action key contract between prompt and evaluator, and guard JSON parsing.

Line 12 asks the model for Actions, but Line 65 reads Content.Action. This mismatch can trigger KeyError or systematic mis-scoring when the model follows the prompt.

Suggested fix

 SYSTEM_PROMPT = (
@@
-    "с ключами 'MessageText' и 'Actions', содержащими как минимум одно "
+    "с ключами 'MessageText' и 'Action', содержащими как минимум одно "
@@
-        if json.loads(response[0].outputs[0].text)["Content"]["Action"] == answers[number]:
+        try:
+            payload = json.loads(response[0].outputs[0].text)
+            predicted_action = payload.get("Content", {}).get("Action")
+        except (json.JSONDecodeError, TypeError):
+            predicted_action = None
+        if predicted_action == answers[number]:

Also applies to: 65-65

🧰 Tools

🪛 Ruff (0.15.15)

[warning] 10-10: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)

---

[warning] 11-11: String contains ambiguous У (CYRILLIC CAPITAL LETTER U). Did you mean Y (LATIN CAPITAL LETTER Y)?

(RUF001)

---

[warning] 12-12: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

[warning] 15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

(RUF001)

---

[warning] 15-15: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?

(RUF001)

---

[warning] 16-16: String contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF001)

🤖 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 `@testing_model/vllm_test.py` around lines 9 - 17, The SYSTEM_PROMPT defines
the contract asking the model to return JSON with an 'Actions' key, but the
evaluator code at line 65 reads from 'Content.Action' (singular), causing a
mismatch that leads to KeyError or scoring failures when the model follows the
prompt. Align the key name between the SYSTEM_PROMPT (lines 9-17) and the
evaluator code at line 65 so they both reference the same key, and add
guard/error handling around the JSON key access to safely handle cases where the
expected keys are missing or misnamed.