refactor: remove unnecessary whitespace#42
Conversation
Blank lines should not contain any tabs or spaces.
|
Here's the code health analysis summary for commits Analysis Summary
|
Resolved issues in the following files with DeepSource Autofix: 1. scripts/legacy/comprehensive_model_validation.py 2. scripts/legacy/deep_model_analysis.py
There was a problem hiding this comment.
Pull Request Overview
This PR removes unnecessary whitespace from multiple Python files to improve code quality by cleaning up blank lines that contain tabs or spaces.
- Removes whitespace-only lines from empty test files
- Updates f-string print statements to use regular string concatenation for static text
- Eliminates trailing whitespace from blank lines in analysis scripts
Reviewed Changes
Copilot reviewed 6 out of 113 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/testing/simple_rate_limiter_test.py | Removes whitespace-only line from empty file |
| scripts/testing/debug_rate_limiter_test.py | Removes whitespace-only line from empty file |
| scripts/legacy/deep_model_analysis.py | Converts f-string print statements to regular strings and removes trailing whitespace |
| scripts/legacy/comprehensive_model_validation.py | Converts f-string print statements to regular strings for static text |
| debug_ci_timing.py | Removes whitespace-only line from empty file |
| debug_ci_robust.py | Removes whitespace-only line from empty file |
| print(f"\n📊 EMOTION MAPPING ANALYSIS") | ||
|
|
||
| print("\n📊 EMOTION MAPPING ANALYSIS") |
There was a problem hiding this comment.
[nitpick] Converting from f-string to regular string for static text is correct, but consider that f-strings are generally preferred in modern Python for consistency, even with static strings. However, this change improves performance slightly by avoiding unnecessary f-string overhead.
|
/gemini review |
|
/sourcery-ai review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive cleanup by removing unnecessary whitespace from blank lines across a large number of files. This is a valuable refactoring that improves code style and consistency throughout the repository. The changes are correct and well-executed. I have no further comments or suggestions.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis update is a large-scale codebase cleanup focused almost entirely on formatting and whitespace adjustments. It removes trailing spaces, extraneous blank lines, and enforces consistent code style across numerous scripts, modules, and notebooks. Additionally, several obsolete or debugging-related files are deleted. One file, Changes
Sequence Diagram(s)Enhanced Bulletproof Training FlowsequenceDiagram
participant User
participant Main
participant EnvironmentValidator
participant DataPreparer
participant Trainer
participant Logger
User->>Main: Run script
Main->>EnvironmentValidator: validate_environment()
EnvironmentValidator-->>Main: Success/Failure
Main->>DataPreparer: prepare_filtered_data()
DataPreparer-->>Main: Filtered data or None
Main->>Trainer: train_model_simple()
loop For each batch
Trainer->>Trainer: Validate batch structure & label ranges
alt Invalid batch
Trainer->>Logger: Log warning, skip batch
else Valid batch
Trainer->>Trainer: Train on batch (try-except)
alt Exception
Trainer->>Logger: Log error, skip batch
end
end
end
Trainer-->>Main: Training result
Main->>Logger: Log completion or error
Notebook Training Function Enhancement (Regex Transformation)sequenceDiagram
participant Script
participant Notebook
participant RegexEngine
Script->>Notebook: Load notebook JSON
Script->>RegexEngine: Apply regex substitutions to training function cell
RegexEngine-->>Notebook: Modified cell with GPU, scheduler, mixed-precision, early stopping
Script->>Notebook: Save updated notebook
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (13)
scripts/legacy/create_unique_fallback_dataset.py (1)
233-237: Align the success message with the duplicate check outcomeIf duplicates are found, the function returns None but the script still prints a success message. Reflect the outcome.
- create_unique_fallback_dataset() - print("\n🎉 Unique fallback dataset created successfully!") + result = create_unique_fallback_dataset() + if result is None: + print("\n⚠️ Failed to create dataset due to duplicates.") + # Optionally: sys.exit(1) + else: + print("\n🎉 Unique fallback dataset created successfully!")scripts/legacy/reorganize_model_directory.py (1)
237-246: Handle broken symlinks with lexists to avoid failuresIf a broken symlink exists at deployment/model, os.path.exists(...) returns False, so the stale link remains and os.symlink(...) will fail with “File exists.” Use os.path.lexists(...) so broken links are detected and unlinked.
- if os.path.exists(symlink_path): + if os.path.lexists(symlink_path): if os.path.islink(symlink_path): os.unlink(symlink_path) else: # Backup the original model directory backup_path = "deployment/model_backup" if os.path.exists(backup_path): shutil.rmtree(backup_path) shutil.move(symlink_path, backup_path) print(f"✅ Backed up original model to: {backup_path}")scripts/legacy/finalize_emotion_model.py (1)
187-191: Critical: incorrect unpacking fromcreate_bert_emotion_classifier().That factory returns
(model, loss_function), not a tokenizer. Unpacking into(model, tokenizer)is wrong and will propagate incorrect types tocreate_augmented_dataset(...).- model, tokenizer = create_bert_emotion_classifier() + model, _ = create_bert_emotion_classifier()If you truly need a tokenizer here, initialize it explicitly:
tokenizer = AutoTokenizer.from_pretrained("bert-base-uncased")scripts/maintenance/fix_code_quality.py (1)
57-66: fix_f_strings corrupts valid f-strings (data-loss bug)Patterns at Line 60/61 will strip the f-prefix from all f-strings, including those with placeholders. The follow-up “invalid syntax” fix can’t recover because the f-prefix is already removed.
Use patterns that only demote f-strings with no placeholders, or drop this risky transform entirely.
- # Fix f-strings without placeholders - content = re.sub(r'f"([^"]*)"', r'"\1"', content) - content = re.sub(r"f'([^']*)'", r"'\1'", content) - - # Fix f-strings with invalid syntax - content = re.sub(r'f"([^"]*)\{([^}]*)\}([^"]*)"', r'f"\1{\2}\3"', content) + # Only demote f-strings that have no braces/placeholders + content = re.sub(r'f"([^"{}]*)"', r'"\1"', content) + content = re.sub(r"f'([^'{}]*)'", r"'\1'", content) + # Avoid attempting to "fix" arbitrary f-string syntax via regex; leave to linters/formatters.scripts/maintenance/fix_remaining_linting.py (3)
104-124: Import sorting moves comments and can mangle imports
- Lines 113-117 treat “#” comments as import lines; don’t.
- Re-assembly doesn’t insert a separator or preserve groups.
- if (stripped.startswith('import ') or - stripped.startswith('from ') or - stripped.startswith('#')): + if (stripped.startswith('import ') + or stripped.startswith('from ')): import_lines.append(line) else: non_import_lines.append(line) - import_lines.sort() + import_lines.sort() - new_content = '\n'.join(import_lines + non_import_lines) + # Separate imports from code + new_content = '\n'.join([*import_lines, '', *non_import_lines])Consider using isort for correctness.
198-208: Directory existence check is ineffectiveLine 206 uses if Path(directory): which is always truthy. Use exists()/is_dir().
- for directory in directories: - if Path(directory): - self.process_directory(directory) + for directory in directories: + p = Path(directory) + if p.exists() and p.is_dir(): + self.process_directory(str(p))
175-187: Broken regex in fix_minor_issuesPattern at Line 179 appears malformed and the replacement duplicates noqa tags.
Either remove this mutation or constrain it precisely:
- pattern = r'TEST_USER_PASSWORD_HASH = "test_hashed_password_123" # noqa: S105]*)"' - replacement = r'TEST_USER_PASSWORD_HASH = "test_hashed_password_123" # noqa: S105 # noqa: S105' + pattern = r'^(TEST_USER_PASSWORD_HASH\s*=\s*)"[^"]*"(.*)$' + replacement = r'\1"test_hashed_password_123"\2 # noqa: S105'Add re.MULTILINE and only apply in test fixtures, not globally.
scripts/testing/final_temperature_test.py (1)
156-166: “Raw probs” printed for wrong samples (last-batch only)probabilities refers to the last batch (Line 165). You’re indexing it with i from the full dataset, so it doesn’t align with test_texts.
Accumulate probs alongside predictions/labels and reference by global index:
- all_predictions = [] - all_labels = [] + all_predictions = [] + all_labels = [] + all_probabilities = [] @@ - probabilities = torch.sigmoid(outputs / temp) + probabilities = torch.sigmoid(outputs / temp) @@ - # Convert to numpy for sklearn + # Convert to numpy for sklearn/printing all_predictions.append(predictions.cpu().numpy()) all_labels.append(labels.cpu().numpy()) + all_probabilities.append(probabilities.cpu().numpy()) @@ - all_predictions = np.concatenate(all_predictions, axis=0) - all_labels = np.concatenate(all_labels, axis=0) + all_predictions = np.concatenate(all_predictions, axis=0) + all_labels = np.concatenate(all_labels, axis=0) + all_probabilities = np.concatenate(all_probabilities, axis=0) @@ - logging.info(f" Raw probs: {probabilities[i].cpu().numpy()}") + logging.info(f" Raw probs: {all_probabilities[i]}")src/api_rate_limiter.py (2)
403-433: Critical: concurrent request counters are never released in middleware
allow_requestincrementsconcurrent_requests, butrelease_requestis never called. This leaks slots and will eventually block all traffic.Apply this fix to call
release_requestin a finally block and still attach headers:@app.middleware("http") async def rate_limit_middleware(request: Request, call_next): """Rate limiting middleware.""" client_ip = request.client.host if request.client else "unknown" user_agent = request.headers.get("user-agent", "") @@ # Check rate limit - allowed, reason, meta = rate_limiter.allow_request(client_ip, user_agent) - - if not allowed: - return JSONResponse( - status_code=429, - content={ - "error": "Rate limit exceeded", - "message": reason, - "retry_after": meta.get("retry_after", 60) - } - ) - - # Add rate limit headers - response = await call_next(request) - response.headers["X-RateLimit-Limit"] = str(config.requests_per_minute) - response.headers["X-RateLimit-Remaining"] = str(meta.get("tokens_remaining", 0)) - response.headers["X-RateLimit-Reset"] = str(meta.get("reset_time", 0)) - - return response + allowed, reason, meta = rate_limiter.allow_request(client_ip, user_agent) + + if not allowed: + return JSONResponse( + status_code=429, + content={ + "error": "Rate limit exceeded", + "message": reason, + "retry_after": meta.get("retry_after", 60), + }, + ) + + try: + response = await call_next(request) + finally: + # Always release the slot after request completes + rate_limiter.release_request(client_ip, user_agent) + + # Add rate limit headers + response.headers["X-RateLimit-Limit"] = str(config.requests_per_minute) + response.headers["X-RateLimit-Remaining"] = str(meta.get("tokens_remaining", 0)) + if "reset_time" in meta: + response.headers["X-RateLimit-Reset"] = str(meta["reset_time"]) + return response
1-433: Ensure every allowed request releases its slot
Therate_limit_middlewareinsrc/api_rate_limiter.pycallsallow_request()(which incrementsconcurrent_requests) but never callsrelease_request(), causing slots to leak. Wrapcall_next()in atry/finallyso thatrelease_request(client_ip, user_agent)is always invoked—even on errors.Locations to update:
- src/api_rate_limiter.py, inside
rate_limit_middleware
- Before returning the response (or on exception), add:
try: response = await call_next(request) finally: rate_limiter.release_request(client_ip, user_agent)- Then set rate-limit headers on
responseand return it.This guarantees that every successful or failed request frees its concurrent slot.
scripts/training/final_combined_training.py (2)
197-218: Fix invalid TrainingArguments parameter: useevaluation_strategy(noteval_strategy)Hugging Face Trainer expects
evaluation_strategy.eval_strategywill be ignored or cause an error depending on version.Apply:
- eval_strategy="steps", + evaluation_strategy="steps",
245-267: Ensure model/input device alignment during ad-hoc testingAfter training on GPU,
modellikely resides on CUDA. The ad-hoc test creates CPU tensors and can error. Move inputs tomodel.deviceor move model to CPU.Minimal fix:
- model.eval() + model.eval() + device = next(model.parameters()).device @@ - inputs = tokenizer(text, return_tensors="pt", truncation=True, max_length=128) - outputs = model(**inputs) + inputs = tokenizer(text, return_tensors="pt", truncation=True, max_length=128) + inputs = {k: v.to(device) for k, v in inputs.items()} + outputs = model(**inputs)scripts/training/comprehensive_domain_adaptation_training.py (1)
523-571: DomainAdaptedEmotionClassifier must subclass nn.Module (critical for .to(), train/eval, etc.)Current class isn’t an nn.Module yet
ModelManagercallsself.model.to(self.device). This will fail at runtime.Minimal fix (follow-up PR recommended):
- class DomainAdaptedEmotionClassifier: + from torch import nn + class DomainAdaptedEmotionClassifier(nn.Module): def __init__(self, model_name="bert-base-uncased", num_labels=None, dropout=0.3): - import torch.nn as nn + # nn imported above from transformers import AutoModel self.bert = AutoModel.from_pretrained(model_name) self.dropout = nn.Dropout(dropout) self.classifier = nn.Linear(self.bert.config.hidden_size, num_labels) self.domain_classifier = nn.Sequential( nn.Linear(self.bert.config.hidden_size, 512), nn.ReLU(), nn.Dropout(0.3), nn.Linear(512, 2) )
♻️ Duplicate comments (1)
scripts/legacy/deep_model_analysis.py (1)
31-31: Converting static f-strings to plain strings is fine.Repeating earlier feedback: using plain strings for static text removes unnecessary f-prefix.
| with open('data/unique_fallback_dataset.json', 'w') as f: | ||
| json.dump(unique_samples, f, indent=2) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure output directory exists before writing the JSON file
If data/ doesn’t exist, open(...) will raise FileNotFoundError. Create the directory first.
Apply within this hunk:
- with open('data/unique_fallback_dataset.json', 'w') as f:
+ os.makedirs('data', exist_ok=True)
+ with open('data/unique_fallback_dataset.json', 'w') as f:Also add the missing import near the top of the file:
import os🤖 Prompt for AI Agents
In scripts/legacy/create_unique_fallback_dataset.py around lines 216 to 217,
before opening the file for writing, check if the 'data/' directory exists and
create it if it does not using os.makedirs with exist_ok=True. Also, add 'import
os' near the top of the file to enable directory operations. This will prevent
FileNotFoundError when the directory is missing.
| f"{template} {random.choice(['It\'s been a long day.', 'Things are going well.', 'I need to process this.', 'This is important to me.'])}", | ||
| f"{template} {random.choice(['I hope this continues.', 'I wonder what\'s next.', 'This feels right.', 'I\'m processing this.'])}", | ||
| f"{template} {random.choice(['I should reflect on this.', 'This is meaningful.', 'I appreciate this moment.', 'I\'m learning from this.'])}" |
There was a problem hiding this comment.
Fix f-string escape sequence syntax issue
The f-strings contain backslash escape sequences which cause syntax errors in Python 3.9. This syntax was only added in Python 3.12.
- f"{template} {random.choice(['It\'s been a long day.', 'Things are going well.', 'I need to process this.', 'This is important to me.'])}",
- f"{template} {random.choice(['I hope this continues.', 'I wonder what\'s next.', 'This feels right.', 'I\'m processing this.'])}",
- f"{template} {random.choice(['I should reflect on this.', 'This is meaningful.', 'I appreciate this moment.', 'I\'m learning from this.'])}"
+ f"{template} {random.choice(['It\\'s been a long day.', 'Things are going well.', 'I need to process this.', 'This is important to me.'])}",
+ f"{template} {random.choice(['I hope this continues.', 'I wonder what\\'s next.', 'This feels right.', 'I\\'m processing this.'])}",
+ f"{template} {random.choice(['I should reflect on this.', 'This is meaningful.', 'I appreciate this moment.', 'I\\'m learning from this.'])}"Or better yet, avoid the issue entirely by using regular string concatenation or format():
- f"{template} {random.choice(['It\'s been a long day.', 'Things are going well.', 'I need to process this.', 'This is important to me.'])}",
+ template + " " + random.choice(['It\'s been a long day.', 'Things are going well.', 'I need to process this.', 'This is important to me.']),Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.12.2)
234-234: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
234-234: SyntaxError: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
235-235: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
235-235: SyntaxError: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
235-235: SyntaxError: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
236-236: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
236-236: SyntaxError: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
🤖 Prompt for AI Agents
In scripts/legacy/expand_journal_dataset.py around lines 234 to 236, the
f-strings use backslash escape sequences inside the expressions, which causes
syntax errors in Python 3.9. To fix this, replace the f-strings with either
regular string concatenation or use the format() method to avoid backslash
escapes inside the expressions, ensuring compatibility with Python 3.9.
| # Calculate F1 score | ||
| val_predictions = torch.cat(val_predictions, dim=0) | ||
| val_labels = torch.cat(val_labels, dim=0) | ||
|
|
||
| f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure sklearn gets NumPy arrays, not torch tensors.
f1_score should receive NumPy arrays. Convert after concatenation.
- val_predictions = torch.cat(val_predictions, dim=0)
- val_labels = torch.cat(val_labels, dim=0)
- f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0)
+ val_predictions = torch.cat(val_predictions, dim=0).cpu().numpy().astype(int)
+ val_labels = torch.cat(val_labels, dim=0).cpu().numpy().astype(int)
+ f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0)I've added a quick search to confirm how val_predictions and val_labels are assembled and whether they reside on GPU:
🏁 Script executed:
#!/bin/bash
rg -n "val_predictions" -A 10 -B 10 scripts/legacy/finalize_emotion_model.pyLength of output: 1402
Convert concatenated tensors to NumPy arrays before calling f1_score
Ensure that sklearn.metrics.f1_score receives NumPy arrays (and integer labels) rather than raw PyTorch tensors:
• File: scripts/legacy/finalize_emotion_model.py
• Lines: 239–244
- val_predictions = torch.cat(val_predictions, dim=0)
- val_labels = torch.cat(val_labels, dim=0)
- f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0)
+ val_predictions = torch.cat(val_predictions, dim=0).cpu().numpy().astype(int)
+ val_labels = torch.cat(val_labels, dim=0).cpu().numpy().astype(int)
+ f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0)📝 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.
| # Calculate F1 score | |
| val_predictions = torch.cat(val_predictions, dim=0) | |
| val_labels = torch.cat(val_labels, dim=0) | |
| f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0) | |
| # Calculate F1 score | |
| val_predictions = torch.cat(val_predictions, dim=0).cpu().numpy().astype(int) | |
| val_labels = torch.cat(val_labels, dim=0).cpu().numpy().astype(int) | |
| f1 = f1_score(val_labels, val_predictions, average='micro', zero_division=0) |
🤖 Prompt for AI Agents
In scripts/legacy/finalize_emotion_model.py around lines 239 to 244, the
concatenated PyTorch tensors val_labels and val_predictions need to be converted
to NumPy arrays with integer type before passing to sklearn.metrics.f1_score.
Fix this by calling .cpu().numpy() on each tensor and converting them to
integers to ensure f1_score receives the correct input format.
| # Calculate metrics | ||
| micro_f1 = f1_score(labels, predictions, average='micro', zero_division=0) | ||
| macro_f1 = f1_score(labels, predictions, average='macro', zero_division=0) | ||
| precision, recall, _, _ = precision_recall_fscore_support( | ||
| labels, predictions, average='micro', zero_division=0 | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same conversion needed in ensemble evaluation.
Convert tensors to NumPy before computing metrics to avoid type/dtype surprises.
- predictions = torch.cat(predictions, dim=0)
- labels = torch.cat(labels, dim=0)
+ predictions = torch.cat(predictions, dim=0).cpu().numpy().astype(int)
+ labels = torch.cat(labels, dim=0).cpu().numpy().astype(int)📝 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.
| # Calculate metrics | |
| micro_f1 = f1_score(labels, predictions, average='micro', zero_division=0) | |
| macro_f1 = f1_score(labels, predictions, average='macro', zero_division=0) | |
| precision, recall, _, _ = precision_recall_fscore_support( | |
| labels, predictions, average='micro', zero_division=0 | |
| ) | |
| # Aggregate predictions and labels, then convert to NumPy ints | |
| predictions = torch.cat(predictions, dim=0).cpu().numpy().astype(int) | |
| labels = torch.cat(labels, dim=0).cpu().numpy().astype(int) | |
| # Calculate metrics | |
| micro_f1 = f1_score(labels, predictions, average='micro', zero_division=0) | |
| macro_f1 = f1_score(labels, predictions, average='macro', zero_division=0) | |
| precision, recall, _, _ = precision_recall_fscore_support( | |
| labels, predictions, average='micro', zero_division=0 | |
| ) |
🤖 Prompt for AI Agents
In scripts/legacy/finalize_emotion_model.py around lines 327 to 333, the labels
and predictions are used directly as tensors in metric calculations, which can
cause type or dtype issues. Convert both labels and predictions tensors to NumPy
arrays before passing them to f1_score and precision_recall_fscore_support
functions to ensure compatibility and avoid errors.
| data_loader = GoEmotionsDataLoader() | ||
| test_data = data_loader.get_test_data() | ||
| _, tokenizer = create_bert_emotion_classifier() | ||
|
|
||
| metrics = evaluate_ensemble(ensemble, test_data, tokenizer, device) |
There was a problem hiding this comment.
Critical: repeated incorrect unpacking; also tokenizer unused.
Same issue: create_bert_emotion_classifier() does not return a tokenizer. Moreover, evaluate_ensemble(...) doesn’t use tokenizer at all.
- _, tokenizer = create_bert_emotion_classifier()
- metrics = evaluate_ensemble(ensemble, test_data, tokenizer, device)
+ _, _ = create_bert_emotion_classifier()
+ metrics = evaluate_ensemble(ensemble, test_data, device=device, tokenizer=None)And simplify evaluate_ensemble signature to drop unused tokenizer param (see next comment).
📝 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.
| data_loader = GoEmotionsDataLoader() | |
| test_data = data_loader.get_test_data() | |
| _, tokenizer = create_bert_emotion_classifier() | |
| metrics = evaluate_ensemble(ensemble, test_data, tokenizer, device) | |
| data_loader = GoEmotionsDataLoader() | |
| test_data = data_loader.get_test_data() | |
| _, _ = create_bert_emotion_classifier() | |
| metrics = evaluate_ensemble(ensemble, test_data, device=device, tokenizer=None) |
🤖 Prompt for AI Agents
In scripts/legacy/finalize_emotion_model.py around lines 412 to 416, remove the
incorrect unpacking of create_bert_emotion_classifier() since it does not return
a tokenizer, and eliminate the tokenizer variable entirely as it is unused.
Also, update the call to evaluate_ensemble() to remove the tokenizer argument,
and subsequently modify the evaluate_ensemble function signature to drop the
unused tokenizer parameter.
| logger.info(f" Temperature {temp}: F1 = {results.get('f1_score', 'N/A'):.4f}") | ||
|
|
There was a problem hiding this comment.
Guard f-string formatting for missing metrics.
results.get('f1_score', 'N/A') with :.4f will raise if key is absent. Format robustly.
- logger.info(f" Temperature {temp}: F1 = {results.get('f1_score', 'N/A'):.4f}")
+ f1 = results.get('f1_score')
+ f1_str = f"{f1:.4f}" if isinstance(f1, (int, float)) else "N/A"
+ logger.info(f" Temperature {temp}: F1 = {f1_str}")📝 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.
| logger.info(f" Temperature {temp}: F1 = {results.get('f1_score', 'N/A'):.4f}") | |
| f1 = results.get('f1_score') | |
| f1_str = f"{f1:.4f}" if isinstance(f1, (int, float)) else "N/A" | |
| logger.info(f" Temperature {temp}: F1 = {f1_str}") |
🤖 Prompt for AI Agents
In scripts/testing/simple_temperature_test.py around lines 80 to 81, the
f-string formatting uses :.4f on results.get('f1_score', 'N/A'), which raises an
error if 'f1_score' is missing because 'N/A' is a string. Fix this by first
checking if 'f1_score' exists and is a number before formatting; if not present,
log 'N/A' without formatting. This ensures robust formatting without exceptions.
| # Add mixed precision training loop | ||
| training_loop_pattern = r'optimizer\.zero_grad\(\)\s+outputs = model\(input_ids=input_ids, attention_mask=attention_mask\)\s+loss = criterion\(outputs, labels\)\s+loss\.backward\(\)\s+optimizer\.step\(\)' | ||
| training_loop_code = '''optimizer.zero_grad() | ||
| with autocast(): | ||
| outputs = model(input_ids=input_ids, attention_mask=attention_mask) | ||
| loss = criterion(outputs, labels) | ||
|
|
||
| scaler.scale(loss).backward() | ||
| scaler.step(optimizer) | ||
| scaler.update()''' | ||
|
|
||
| new_source = re.sub(training_loop_pattern, training_loop_code, new_source) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Regex-based code surgery is brittle; add a quick verification step
Patterns may miss slightly different formatting. Recommend verifying that all intended substitutions occurred before writing.
Add this check before saving:
assert 'GradScaler' in new_source and 'autocast' in new_source and 'scheduler.step' in new_source, "Expected training enhancements not injected"Or run this script to scan the modified cell content after execution:
🏁 Script executed:
#!/bin/bash
rg -n "autocast|GradScaler|scheduler\.step|cudnn\.benchmark|Early stopping triggered" notebooks/expanded_dataset_training_improved.ipynbLength of output: 229
🏁 Script executed:
#!/bin/bash
# List all notebook files in the repository
echo "Notebook files found:"
fd --extension ipynb
echo ""
# Search for mixed-precision and training enhancements in all notebooks
echo "Searching for patterns in notebooks:"
rg -n "autocast|GradScaler|scheduler\.step|cudnn\.benchmark|Early stopping triggered" -g '*.ipynb'Length of output: 4871
Add verification in improve_expanded_training_notebook.py to guard against brittle regex substitutions
Our scan confirms that autocast, GradScaler, and scheduler.step appear in notebooks/training/expanded_dataset_training_improved.ipynb, so the substitution is working—but relying on a single regex is fragile. Please add an assertion right after your re.sub call in scripts/training/improve_expanded_training_notebook.py to fail fast if the injection didn’t occur:
new_source = re.sub(training_loop_pattern, training_loop_code, new_source)
+ # Verify that mixed‐precision and scheduler calls were injected
+ assert "GradScaler" in new_source, "Expected GradScaler injection missing"
+ assert "autocast" in new_source, "Expected autocast context missing"
+ assert "scheduler.step" in new_source, "Expected scheduler.step to remain in the loop"• File needing change:
– scripts/training/improve_expanded_training_notebook.py (after line 105)
This will catch formatting variations or future edits that break your regex.
📝 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.
| # Add mixed precision training loop | |
| training_loop_pattern = r'optimizer\.zero_grad\(\)\s+outputs = model\(input_ids=input_ids, attention_mask=attention_mask\)\s+loss = criterion\(outputs, labels\)\s+loss\.backward\(\)\s+optimizer\.step\(\)' | |
| training_loop_code = '''optimizer.zero_grad() | |
| with autocast(): | |
| outputs = model(input_ids=input_ids, attention_mask=attention_mask) | |
| loss = criterion(outputs, labels) | |
| scaler.scale(loss).backward() | |
| scaler.step(optimizer) | |
| scaler.update()''' | |
| new_source = re.sub(training_loop_pattern, training_loop_code, new_source) | |
| # Add mixed precision training loop | |
| training_loop_pattern = r'optimizer\.zero_grad\(\)\s+outputs = model\(input_ids=input_ids, attention_mask=attention_mask\)\s+loss = criterion\(outputs, labels\)\s+loss\.backward\(\)\s+optimizer\.step\(\)' | |
| training_loop_code = '''optimizer.zero_grad() | |
| with autocast(): | |
| outputs = model(input_ids=input_ids, attention_mask=attention_mask) | |
| loss = criterion(outputs, labels) | |
| scaler.scale(loss).backward() | |
| scaler.step(optimizer) | |
| scaler.update()''' | |
| new_source = re.sub(training_loop_pattern, training_loop_code, new_source) | |
| # Verify that mixed-precision and scheduler calls were injected | |
| assert "GradScaler" in new_source, "Expected GradScaler injection missing" | |
| assert "autocast" in new_source, "Expected autocast context missing" | |
| assert "scheduler.step" in new_source, "Expected scheduler.step to remain in the loop" |
🤖 Prompt for AI Agents
In scripts/training/improve_expanded_training_notebook.py after line 105, add an
assertion immediately following the re.sub call to verify that the substitution
actually occurred. This can be done by checking that the new source code string
differs from the original or by confirming the presence of the injected mixed
precision training loop code. This assertion will cause the script to fail fast
if the regex substitution did not apply, protecting against brittle regex
failures due to formatting changes or future edits.
| # Check if tokens available | ||
| if self.buckets[client_key] < 0.999999: # Use small epsilon to handle floating-point precision | ||
| return False, "Rate limit exceeded", { | ||
| "client_key": client_key, | ||
| "tokens": self.buckets[client_key], | ||
| "rate_limit": self.config.requests_per_minute | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide accurate retry_after (and optionally reset_time) in rate-limit responses
When tokens are exhausted, the 429 body sets a default retry_after=60. Compute and return the actual time until the next token.
- if self.buckets[client_key] < 0.999999: # Use small epsilon to handle floating-point precision
- return False, "Rate limit exceeded", {
- "client_key": client_key,
- "tokens": self.buckets[client_key],
- "rate_limit": self.config.requests_per_minute
- }
+ if self.buckets[client_key] < 0.999999: # Use small epsilon to handle floating-point precision
+ rate_per_sec = self.config.requests_per_minute / 60.0
+ missing = max(0.0, 1.0 - self.buckets[client_key])
+ retry_after = max(0, int(missing / rate_per_sec)) if rate_per_sec > 0 else 60
+ reset_time = int(time.time()) + retry_after
+ return False, "Rate limit exceeded", {
+ "client_key": client_key,
+ "tokens": self.buckets[client_key],
+ "rate_limit": self.config.requests_per_minute,
+ "retry_after": retry_after,
+ "reset_time": reset_time,
+ }If you adopt this, also add import math if you prefer rounding up, or keep the integer cast as shown.
📝 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.
| # Check if tokens available | |
| if self.buckets[client_key] < 0.999999: # Use small epsilon to handle floating-point precision | |
| return False, "Rate limit exceeded", { | |
| "client_key": client_key, | |
| "tokens": self.buckets[client_key], | |
| "rate_limit": self.config.requests_per_minute | |
| } | |
| # Check if tokens available | |
| if self.buckets[client_key] < 0.999999: # Use small epsilon to handle floating-point precision | |
| rate_per_sec = self.config.requests_per_minute / 60.0 | |
| missing = max(0.0, 1.0 - self.buckets[client_key]) | |
| retry_after = max(0, int(missing / rate_per_sec)) if rate_per_sec > 0 else 60 | |
| reset_time = int(time.time()) + retry_after | |
| return False, "Rate limit exceeded", { | |
| "client_key": client_key, | |
| "tokens": self.buckets[client_key], | |
| "rate_limit": self.config.requests_per_minute, | |
| "retry_after": retry_after, | |
| "reset_time": reset_time, | |
| } |
🤖 Prompt for AI Agents
In src/api_rate_limiter.py around lines 301 to 308, the rate-limit response
currently returns a fixed retry_after value of 60 seconds when tokens are
exhausted. Modify the code to calculate the actual retry_after time based on the
token refill rate and the current token count, then include this computed
retry_after value in the response dictionary. Optionally, add a reset_time field
if relevant. Also, add an import for the math module if you choose to use
math.ceil for rounding the retry_after value.
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | ||
| validation_info['loadable'] = True | ||
|
|
||
| # Validate model data structure | ||
| if not isinstance(model_data, dict): | ||
| validation_info['issues'].append("Model file is not a valid state dict") | ||
| else: | ||
| # Check for required keys | ||
| if 'state_dict' not in model_data: | ||
| validation_info['issues'].append("Model file missing state_dict") | ||
|
|
||
| if 'config' not in model_data: | ||
| validation_info['issues'].append("Model file missing config") | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add backward-compatible torch.load fallback for weights_only.
weights_only is not supported in older PyTorch (1.x). Fallback improves compatibility with your own version matrix.
- model_data = torch.load(model_path, map_location='cpu', weights_only=True)
+ try:
+ model_data = torch.load(model_path, map_location='cpu', weights_only=True)
+ except TypeError:
+ # Fallback for PyTorch < 2.0
+ model_data = torch.load(model_path, map_location='cpu')📝 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.
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | |
| validation_info['loadable'] = True | |
| # Validate model data structure | |
| if not isinstance(model_data, dict): | |
| validation_info['issues'].append("Model file is not a valid state dict") | |
| else: | |
| # Check for required keys | |
| if 'state_dict' not in model_data: | |
| validation_info['issues'].append("Model file missing state_dict") | |
| if 'config' not in model_data: | |
| validation_info['issues'].append("Model file missing config") | |
| except Exception as e: | |
| try: | |
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | |
| except TypeError: | |
| # Fallback for PyTorch < 2.0 | |
| model_data = torch.load(model_path, map_location='cpu') | |
| validation_info['loadable'] = True | |
| # Validate model data structure | |
| if not isinstance(model_data, dict): | |
| validation_info['issues'].append("Model file is not a valid state dict") | |
| else: | |
| # Check for required keys | |
| if 'state_dict' not in model_data: | |
| validation_info['issues'].append("Model file missing state_dict") | |
| if 'config' not in model_data: | |
| validation_info['issues'].append("Model file missing config") | |
| except Exception as e: |
🤖 Prompt for AI Agents
In src/models/secure_loader/model_validator.py around lines 197 to 211, the
torch.load call uses the weights_only parameter which is unsupported in older
PyTorch versions. To fix this, add a try-except block that first attempts to
load with weights_only=True, and if it raises a TypeError, retry loading without
the weights_only argument. This fallback ensures compatibility with older
PyTorch versions while maintaining the intended behavior.
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | ||
|
|
||
| # Filter model_config to only include valid constructor parameters | ||
| import inspect | ||
| constructor_params = inspect.signature(model_class.__init__).parameters | ||
| valid_params = {k: v for k, v in model_config.items() if k in constructor_params} | ||
| model = model_class(**valid_params) | ||
|
|
||
| if 'state_dict' in model_data: | ||
| model.load_state_dict(model_data['state_dict']) | ||
|
|
||
| structure_valid, structure_info = self.validate_model_structure(model) | ||
| comprehensive_info['structure_validation'] = structure_info | ||
| if not structure_valid: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mirror torch.load fallback during comprehensive load as well.
Keeps behavior consistent across validation paths.
- model_data = torch.load(model_path, map_location='cpu', weights_only=True)
+ try:
+ model_data = torch.load(model_path, map_location='cpu', weights_only=True)
+ except TypeError:
+ model_data = torch.load(model_path, map_location='cpu')📝 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.
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | |
| # Filter model_config to only include valid constructor parameters | |
| import inspect | |
| constructor_params = inspect.signature(model_class.__init__).parameters | |
| valid_params = {k: v for k, v in model_config.items() if k in constructor_params} | |
| model = model_class(**valid_params) | |
| if 'state_dict' in model_data: | |
| model.load_state_dict(model_data['state_dict']) | |
| structure_valid, structure_info = self.validate_model_structure(model) | |
| comprehensive_info['structure_validation'] = structure_info | |
| if not structure_valid: | |
| try: | |
| model_data = torch.load(model_path, map_location='cpu', weights_only=True) | |
| except TypeError: | |
| model_data = torch.load(model_path, map_location='cpu') | |
| # Filter model_config to only include valid constructor parameters | |
| import inspect | |
| constructor_params = inspect.signature(model_class.__init__).parameters | |
| valid_params = {k: v for k, v in model_config.items() if k in constructor_params} | |
| model = model_class(**valid_params) | |
| if 'state_dict' in model_data: | |
| model.load_state_dict(model_data['state_dict']) | |
| structure_valid, structure_info = self.validate_model_structure(model) | |
| comprehensive_info['structure_validation'] = structure_info | |
| if not structure_valid: |
🤖 Prompt for AI Agents
In src/models/secure_loader/model_validator.py around lines 373 to 386, the
torch.load call during comprehensive load does not use the fallback parameter
like in other validation paths, causing inconsistent behavior. Modify the
torch.load invocation to include the fallback argument as done elsewhere to
ensure consistent loading behavior across all validation paths.
Blank lines should not contain any tabs or spaces.
Summary by CodeRabbit
Refactor
Chores
New Features
Documentation