Open
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[config] is_auto_command: True
is_new_pr: True
model_reasoning: vertex_ai/gemini-2.5-pro
model: gpt-5.2-2025-12-11
model_turbo: anthropic/claude-haiku-4-5-20251001
fallback_models: ['anthropic/claude-sonnet-4-5-20250929', 'bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0']
second_model_for_exhaustive_mode: o4-mini
git_provider: github
publish_output: True
publish_output_no_suggestions: True
publish_output_progress: True
verbosity_level: 0
publish_logs: False
debug_mode: False
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
use_global_wiki_settings_file: False
disable_auto_feedback: False
ai_timeout: 150
response_language: en-US
clone_repo_instead_of_fetch: True
always_clone: False
add_repo_metadata: True
clone_repo_time_limit: 300
publish_inline_comments_fallback_batch_size: 5
publish_inline_comments_fallback_sleep_time: 2
max_model_tokens: 32000
custom_model_max_tokens: -1
patch_extension_skip_types: ['.md', '.txt']
extra_allowed_extensions: []
allow_dynamic_context: True
allow_forward_dynamic_context: True
max_extra_lines_before_dynamic_context: 12
patch_extra_lines_before: 5
patch_extra_lines_after: 1
ai_handler: litellm
cli_mode: False
TRIAL_GIT_ORG_MAX_INVOKES_PER_MONTH: 75
TRIAL_RATIO_CLOSE_TO_LIMIT: 0.8
invite_only_mode: False
enable_request_access_msg_on_new_pr: False
check_also_invites_field: False
calculate_context: True
disable_checkboxes: False
output_relevant_configurations: True
large_patch_policy: clip
seed: -1
temperature: 0.2
allow_dynamic_context_ab_testing: False
choose_dynamic_context_ab_testing_ratio: 0.5
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^\\[Bump\\]']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: ['do-not-merge', 'skip-qodo']
ignore_ticket_labels: []
allow_only_specific_folders: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
enable_ai_metadata: True
present_reasoning: True
max_tickets: 10
max_tickets_chars: 8000
prevent_any_approval: False
enable_comment_approval: False
enable_auto_approval: False
auto_approve_for_low_review_effort: -1
auto_approve_for_no_suggestions: False
ensure_ticket_compliance: False
new_diff_format: True
new_diff_format_add_external_references: True
enable_custom_labels: False
[pr_reviewer] require_score_review: True
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: True
require_security_review: True
require_todo_scan: True
require_ticket_analysis_review: True
require_ticket_labels: False
require_no_ticket_labels: False
check_pr_additional_content: False
persistent_comment: True
extra_instructions: You are a Senior Technical Architect specializing in the Frappe Framework, Python, and MariaDB optimization.
Be concise but strict about performance and security. Provide code snippets (or pseudocode) for fixes.
========================
1) 🚨 FRAPPE PERFORMANCE KILLERS (High Priority)
========================
A) N+1 Query Detection (must flag):
- Flag ANY loop that contains `frappe.get_doc`, `frappe.db.get_value`, `frappe.db.get_all/get_list`, or `frappe.db.sql`.
- Explain the query multiplier (e.g., “1 + N queries”) and propose a bulk-fetch refactor:
- pre-fetch with `frappe.get_all/get_list` (with fields + filters) or one SQL query with IN,
- build a dict/map keyed by name, then iterate without DB calls.
B) Heavy Writes / Controller chain (must flag):
- If `doc.save()` is used to update just 1–2 fields, aggressively suggest replacing with:
- `frappe.db.set_value(doctype, name, field, value)`
- or `frappe.db.set_value(doctype, name, {field1: v1, field2: v2})`
- Explain it avoids triggering heavy validations / hooks / controller logic unnecessarily.
- Exception: if controller validations are required for correctness, require justification.
C) ORM abuse / wrong API choice:
- Warn against `frappe.db.sql()` if the same can be achieved by permission-safe ORM (`frappe.get_list/get_all`)
unless it is a complex join/aggregation needing SQL.
- Also warn when `frappe.db.*` is used in user-facing endpoints where permission checks matter.
D) Redundant commits (must flag):
- `frappe.db.commit()` inside loops is forbidden unless there is a carefully justified transactional reason.
- Suggest moving commit outside loop, batching, or using background jobs.
E) Unbounded reads:
- Flag `get_all/get_list` without strong filters or without pagination on potentially large tables.
- Recommend adding filters, indexed ordering, `limit/page_length`, or background processing.
F) Hot hooks:
- Flag heavy work (bulk queries, file IO, network calls) inside validate/on_update/after_insert hooks.
- Recommend `frappe.enqueue` and async jobs.
========================
2) 🛡️ SECURITY & SQL SAFETY (must be strict)
========================
A) SQL injection:
- Strictly forbid f-strings / concatenation inside `frappe.db.sql()`.
- REQUIRE parameter passing: `frappe.db.sql("... %s", (val,))`.
B) Whitelisting:
- If functions are intended to be called from client (JS / REST), ensure `@frappe.whitelist()` exists.
- Confirm sensitive functions are NOT accidentally exposed.
C) Permission checks:
- For new `get_list/get_all`/SQL calls, ensure strict permissions are not bypassed.
- If `ignore_permissions=True` or `frappe.db.*` is used, REQUIRE explicit justification + risk assessment.
D) Unsafe patterns:
- Flag `eval/exec`, insecure deserialization, and unsafe file path usage.
========================
3) 🐍 PYTHONIC & LOGIC OPTIMIZATIONS
========================
A) Mutable defaults (must flag):
- `def f(x=[])` / `def f(x={})` etc. Require `None` + initialization.
B) Data structure choices:
- If code does `if item in big_list`, suggest `set(big_list)` for O(1) lookups.
C) Inefficient loops:
- Reduce nested loops; pull invariants out; memoize expensive computations.
D) List building:
- Suggest list comprehensions ONLY when readability remains good.
- Avoid creating large intermediate lists when generators/chunking is better.
E) String + datetime churn:
- Flag repeated parsing/formatting/conversions inside loops.
========================
4) 📉 MARIADB / DATABASE OPTIMIZATION CHECKS
========================
A) Wildcards / over-fetching:
- Warn against `SELECT *`. Require selecting specific columns.
B) Indexing:
- If a field is used in WHERE / JOIN / ORDER BY patterns frequently, ask if it needs an index.
- If appropriate, suggest composite index order aligned with query filters (e.g., company + docstatus + posting_date).
C) Sorting + limiting:
- Flag ORDER BY on large sets without indexed order; recommend index or reduce rows first.
D) Aggregations:
- If Python loops compute SUM/COUNT over many rows, suggest SQL aggregation where safe.
========================
5) OUTPUT REQUIREMENTS
========================
- Provide concrete refactor steps and code snippets.
- Quantify impact when possible: “reduces from N queries to 1”.
- If table sizes/indexes are unknown, recommend checking EXPLAIN and adding indexes if confirmed.
final_update_message: False
enable_review_labels_security: True
enable_review_labels_effort: True
enable_help_text: False
num_max_findings: 8
|
PR Code Suggestions ✨Explore these optional code suggestions:
🛠️ Relevant configurations:
[config] is_auto_command: True
is_new_pr: True
model_reasoning: vertex_ai/gemini-2.5-pro
model: gpt-5.2-2025-12-11
model_turbo: anthropic/claude-haiku-4-5-20251001
fallback_models: ['anthropic/claude-sonnet-4-5-20250929', 'bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0']
second_model_for_exhaustive_mode: o4-mini
git_provider: github
publish_output: True
publish_output_no_suggestions: True
publish_output_progress: True
verbosity_level: 0
publish_logs: False
debug_mode: False
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
use_global_wiki_settings_file: False
disable_auto_feedback: False
ai_timeout: 150
response_language: en-US
clone_repo_instead_of_fetch: True
always_clone: False
add_repo_metadata: True
clone_repo_time_limit: 300
publish_inline_comments_fallback_batch_size: 5
publish_inline_comments_fallback_sleep_time: 2
max_model_tokens: 32000
custom_model_max_tokens: -1
patch_extension_skip_types: ['.md', '.txt']
extra_allowed_extensions: []
allow_dynamic_context: True
allow_forward_dynamic_context: True
max_extra_lines_before_dynamic_context: 12
patch_extra_lines_before: 5
patch_extra_lines_after: 1
ai_handler: litellm
cli_mode: False
TRIAL_GIT_ORG_MAX_INVOKES_PER_MONTH: 75
TRIAL_RATIO_CLOSE_TO_LIMIT: 0.8
invite_only_mode: False
enable_request_access_msg_on_new_pr: False
check_also_invites_field: False
calculate_context: True
disable_checkboxes: False
output_relevant_configurations: True
large_patch_policy: clip
seed: -1
temperature: 0.2
allow_dynamic_context_ab_testing: False
choose_dynamic_context_ab_testing_ratio: 0.5
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^\\[Bump\\]']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: ['do-not-merge', 'skip-qodo']
ignore_ticket_labels: []
allow_only_specific_folders: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
enable_ai_metadata: True
present_reasoning: True
max_tickets: 10
max_tickets_chars: 8000
prevent_any_approval: False
enable_comment_approval: False
enable_auto_approval: False
auto_approve_for_low_review_effort: -1
auto_approve_for_no_suggestions: False
ensure_ticket_compliance: False
new_diff_format: True
new_diff_format_add_external_references: True
enable_custom_labels: False
extra_statistics: {'suggestion_statistics': [{'score': 7, 'label': 'general'}, {'score': 9, 'label': 'security'}]}
[pr_code_suggestions] suggestions_depth: exhaustive
commitable_code_suggestions: False
decouple_hunks: False
dual_publishing_score_threshold: -1
focus_only_on_problems: True
allow_thumbs_up_down: False
enable_suggestion_type_reuse: False
enable_more_suggestions_checkbox: True
high_level_suggestions_enabled: True
extra_instructions: Focus suggestions on PERFORMANCE + SECURITY:
- Reduce DB roundtrips (bulk fetch, IN queries, joins/aggregates).
- Prefer set_value over save for small field updates unless validations are required.
- Ensure parameterized SQL only.
- Ensure endpoints are whitelisted intentionally and respect permissions.
- Prefer chunking/generators for big datasets.
Return copy-paste ready snippets when feasible.
enable_help_text: False
show_extra_context: False
persistent_comment: True
max_history_len: 5
apply_suggestions_checkbox: True
enable_chat_in_code_suggestions: True
apply_limit_scope: True
suggestions_score_threshold: 0
new_score_mechanism: True
new_score_mechanism_th_high: 9
new_score_mechanism_th_medium: 7
discard_unappliable_suggestions: False
num_code_suggestions_per_chunk: 6
num_best_practice_suggestions: 2
max_number_of_calls: 3
final_clip_factor: 0.8
demand_code_suggestions_self_review: True
code_suggestions_self_review_text: **Author self-review**: I have reviewed the PR code suggestions, and addressed the relevant ones.
approve_pr_on_self_review: False
fold_suggestions_on_self_review: True
publish_post_process_suggestion_impact: True
wiki_page_accepted_suggestions: True
enable_local_self_reflect_in_large_prs: False
simplify_response: True
|
|||||||||||||||||||||
|
Persistent suggestions updated to latest commit 8d3e5e7 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
Enhancement, Bug fix
Description
Refactored
on_submitto use bulk SQL update formanufacturing_order_qtyinstead of individual row updatesImplemented caching mechanism via
get_manufacturing_plan_data()to reduce database queries during manufacturing order creationSimplified BOM validation logic by removing complex BOM type checking and merging duplicate table handling
Removed unused helper functions (
get_mwo,get_sales_order,get_repair_pending_ppo_sales_order) and consolidated intoget_details_to_appendRefactored
get_items_for_production()to use unifieddocs_to_appendfield and optimized MWO data fetching with GROUP BYAdded
.pre-commit-config.yamlfor code quality enforcementUpdated Manufacturing Plan schema to remove
sales_orderandmanufacturing_work_orderchild tablesDiagram Walkthrough
File Walkthrough
manufacturing_plan.py
Bulk update and caching optimization for manufacturing order creationjewellery_erpnext/jewellery_erpnext/doctype/manufacturing_plan/manufacturing_plan.py
update_existing()calls with single bulk SQLUPDATE statement in
on_submit()get_manufacturing_plan_data()method to pre-fetch and cache allrequired document data (SO items, MWO, BOM, Customer, Item records)
create_manufacturing_order()to accept and use cached datamaps instead of individual DB queries per row
validate_qty_with_bom_creation()by removing BOM typevalidation and auto-setting
manufacturing_bom = bomget_items_for_production()to use singledocs_to_appendfield and optimized MWO aggregation with GROUP BY SQL
get_mwo(),get_sales_order(),get_repair_pending_ppo_sales_order(),cancel_bom(),create_new_bom()get_details_to_append(),map_docs(),fetch_doc_map()utils.py
Cleanup utility functions and code formattingjewellery_erpnext/jewellery_erpnext/doctype/manufacturing_plan/doc_events/utils.py
get_mwo()function (consolidated into mainmanufacturing_plan.py)
get_mwo_details()with improved code formatting andf-string to
.format()conversionjsonimportmanufacturing_plan.js
Update method references for consolidated functionsjewellery_erpnext/jewellery_erpnext/doctype/manufacturing_plan/manufacturing_plan.js
get_sales_orderbutton to callget_details_to_appendinsteadof
get_sales_orderget_mwobutton to callget_details_to_appendinstead ofget_mwomap_current_docto use custommap_docsmethod instead offrappe.model.mapper.map_docsmanufacturing_plan.json
Remove child tables and update schema configurationjewellery_erpnext/jewellery_erpnext/doctype/manufacturing_plan/manufacturing_plan.json
sales_orderchild table field and buttonget_items_for_production_ordermanufacturing_work_orderchild table fieldgrid_page_length: 50androw_format: Dynamicconfigurationmodifiedtimestamp to 2025-12-17manufacturing_plan_table.json
Update field constraints and visibility in child tablejewellery_erpnext/jewellery_erpnext/doctype/manufacturing_plan_table/manufacturing_plan_table.json
qty_per_manufacturing_orderfield fromreqd: 1tonon_negative: 1, read_only: 0mwofield fromhidden: 1toread_only: 1(now visible butread-only)
grid_page_length: 50androw_format: Dynamicconfigurationmodifiedtimestamp to 2025-12-18.pre-commit-config.yaml
Add pre-commit hooks for code quality.pre-commit-config.yaml
whitespace, YAML validation, merge conflicts, AST checks
🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]