Upgrade to Opzione A #1
Open
Conversation
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
Reviewer's GuideThis PR upgrades the GIST assessment to Opzione A by fully expanding the questionnaire schema, refactoring the calculation engine into modular methods, overhauling the Flask app with a JSON API and structured routes, revamping frontend templates under a Tailwind-based Jinja layout, and updating dependencies and project documentation. Entity relationship diagram for questionnaire and component weights/benchmarkserDiagram
COMPONENT_WEIGHTS {
physical float
architectural float
security float
compliance float
}
SECTOR_BENCHMARKS {
physical avg float
physical target float
architectural avg float
architectural target float
security avg float
security target float
compliance avg float
compliance target float
}
COMPONENT_LABELS {
physical string
architectural string
security string
compliance string
}
QUESTIONNAIRE {
physical list
architectural list
security list
compliance list
}
QUESTIONNAIRE ||--o{ COMPONENT_WEIGHTS : "weights for"
QUESTIONNAIRE ||--o{ SECTOR_BENCHMARKS : "benchmarks for"
QUESTIONNAIRE ||--o{ COMPONENT_LABELS : "labels for"
Class diagram for the expanded GISTCalculator and questionnaire data modelclassDiagram
class GISTCalculator {
- organization_name: str
- weights: dict
- gamma: float
- benchmarks: dict
+ __init__(organization_name)
+ calculate_score(questionnaire_answers)
- _calculate_component_scores(answers)
- _calculate_gist_formula(component_scores)
- _get_maturity_level(gist_score)
- _generate_recommendations(component_scores)
- _calculate_priority(gap, component)
- _get_action_plan(component, current, target)
- _estimate_effort(gap)
- _estimate_roi(component, gap)
- _calculate_benchmarks(component_scores)
- _score_to_percentile(vs_sector)
}
class QUESTIONNAIRE {
physical: list[Question]
architectural: list[Question]
security: list[Question]
compliance: list[Question]
}
class Question {
id: str
question: str
options: list[Option]
}
class Option {
value: int
text: str
}
GISTCalculator --> "1" QUESTIONNAIRE : uses
QUESTIONNAIRE "1" --> "*" Question
Question "1" --> "*" Option
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables. (link)
General comments:
- Move the hard‐coded app.secret_key in app.py into an environment variable or config file to avoid exposing secrets in source control.
- The GISTCalculator class has grown very large—consider splitting calculation, recommendation logic, and benchmarking into separate modules or helper classes to improve readability and testability.
- questionnaire.py is now a massive static structure—think about extracting the question definitions into a JSON/YAML file or separate module to make updates and maintenance easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Move the hard‐coded app.secret_key in app.py into an environment variable or config file to avoid exposing secrets in source control.
- The GISTCalculator class has grown very large—consider splitting calculation, recommendation logic, and benchmarking into separate modules or helper classes to improve readability and testability.
- questionnaire.py is now a massive static structure—think about extracting the question definitions into a JSON/YAML file or separate module to make updates and maintenance easier.
## Individual Comments
### Comment 1
<location> `gist_calculator.py:64-65` </location>
<code_context>
+ """Calcola score medio 0-100 per ogni componente."""
+ component_scores = {}
+
+ for component, questions in answers.items():
+ if questions:
+ # Media dei valori delle risposte
+ values = list(questions.values())
</code_context>
<issue_to_address>
**issue:** Consider handling missing or malformed question values more robustly.
The code does not validate question values, which may cause errors or incorrect results if values are missing or non-numeric. Add checks or defaults to ensure reliable score calculations.
</issue_to_address>
### Comment 2
<location> `gist_calculator.py:79-84` </location>
<code_context>
+ """
+ gist_score = 0
+
+ for component, score in component_scores.items():
+ if component in self.weights:
+ # Formula con esponente per rendimenti decrescenti
+ weighted_score = self.weights[component] * (score ** self.gamma)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Skip components not present in weights to avoid silent errors.
Consider adding a log message or raising an error when a component is missing from self.weights to prevent silent miscalculations due to typos or unexpected input.
```suggestion
import logging
logger = logging.getLogger(__name__)
gist_score = 0
for component, score in component_scores.items():
if component in self.weights:
# Formula con esponente per rendimenti decrescenti
weighted_score = self.weights[component] * (score ** self.gamma)
else:
logger.warning(f"Component '{component}' not found in weights. Skipping this component.")
continue
```
</issue_to_address>
### Comment 3
<location> `gist_calculator.py:227-229` </location>
<code_context>
}
+ # ROI scala con il gap da colmare
+ roi = base_roi[component] * (gap / 30)
+ return f"{int(roi)}% a 3 anni"
+
</code_context>
<issue_to_address>
**suggestion:** Guard against negative or zero gap values in ROI calculation.
If gap is zero or negative, ROI may be zero or negative, which is likely not meaningful. Consider ensuring ROI is non-negative or handling non-positive gap values explicitly.
```suggestion
# ROI scala con il gap da colmare
if gap <= 0:
return "Nessun ROI calcolabile: gap nullo o negativo"
roi = base_roi[component] * (gap / 30)
roi = max(0, roi)
return f"{int(roi)}% a 3 anni"
```
</issue_to_address>
### Comment 4
<location> `app.py:49-55` </location>
<code_context>
+
+ # Calcola GIST Score
+ calculator = GISTCalculator(company_name)
+ results = calculator.calculate_score(answers)
- score_calcolato = calcola_gist_score(punteggi)
</code_context>
<issue_to_address>
**suggestion:** Consider validating the structure of answers before calculation.
Malformed or incomplete answer data could cause errors or incorrect results. Adding validation will help ensure reliable calculations.
```suggestion
# Validazione base
if not answers:
return jsonify({'error': 'Nessuna risposta fornita'}), 400
def validate_answers_structure(answers):
# Esempio di validazione: answers deve essere un dict con almeno una chiave e valore numerico
if not isinstance(answers, dict):
return False
if not answers:
return False
for key, value in answers.items():
if not isinstance(key, str):
return False
if not isinstance(value, (int, float)):
return False
return True
if not validate_answers_structure(answers):
return jsonify({'error': 'Struttura delle risposte non valida'}), 400
# Calcola GIST Score
calculator = GISTCalculator(company_name)
results = calculator.calculate_score(answers)
```
</issue_to_address>
### Comment 5
<location> `templates/results.html:174` </location>
<code_context>
+{% block extra_scripts %}
+<script>
+// Load results from sessionStorage
+const results = JSON.parse(sessionStorage.getItem('gist_results'));
+
+if (!results) {
</code_context>
<issue_to_address>
**suggestion:** Consider fallback for missing or corrupted sessionStorage data.
Instead of redirecting immediately, show an error message or offer a recovery option to improve user experience.
</issue_to_address>
### Comment 6
<location> `app.py:101` </location>
<code_context>
app.run(debug=True, host='0.0.0.0', port=5000)
</code_context>
<issue_to_address>
**security (python.flask.security.audit.debug-enabled):** Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables.
*Source: opengrep*
</issue_to_address>
### Comment 7
<location> `gist_calculator.py:79-87` </location>
<code_context>
def _calculate_gist_formula(self, component_scores):
"""
Applica formula GIST originale: Σ(w_k * S_k^γ)
"""
gist_score = 0
for component, score in component_scores.items():
if component in self.weights:
# Formula con esponente per rendimenti decrescenti
weighted_score = self.weights[component] * (score ** self.gamma)
gist_score += weighted_score
return gist_score
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Convert for loop into call to sum() ([`sum-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/sum-comprehension/))
- Inline variable that is only used once ([`inline-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-variable/))
- Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
```suggestion
return sum(
self.weights[component] * (score**self.gamma)
for component, score in component_scores.items()
if component in self.weights
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+64
to
+65
| for component, questions in answers.items(): | ||
| if questions: |
There was a problem hiding this comment.
issue: Consider handling missing or malformed question values more robustly.
The code does not validate question values, which may cause errors or incorrect results if values are missing or non-numeric. Add checks or defaults to ensure reliable score calculations.
Comment on lines
+79
to
+84
| gist_score = 0 | ||
|
|
||
| for component, score in component_scores.items(): | ||
| if component in self.weights: | ||
| # Formula con esponente per rendimenti decrescenti | ||
| weighted_score = self.weights[component] * (score ** self.gamma) |
There was a problem hiding this comment.
suggestion (bug_risk): Skip components not present in weights to avoid silent errors.
Consider adding a log message or raising an error when a component is missing from self.weights to prevent silent miscalculations due to typos or unexpected input.
Suggested change
| gist_score = 0 | |
| for component, score in component_scores.items(): | |
| if component in self.weights: | |
| # Formula con esponente per rendimenti decrescenti | |
| weighted_score = self.weights[component] * (score ** self.gamma) | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| gist_score = 0 | |
| for component, score in component_scores.items(): | |
| if component in self.weights: | |
| # Formula con esponente per rendimenti decrescenti | |
| weighted_score = self.weights[component] * (score ** self.gamma) | |
| else: | |
| logger.warning(f"Component '{component}' not found in weights. Skipping this component.") | |
| continue |
Comment on lines
+227
to
+229
| # ROI scala con il gap da colmare | ||
| roi = base_roi[component] * (gap / 30) | ||
| return f"{int(roi)}% a 3 anni" |
There was a problem hiding this comment.
suggestion: Guard against negative or zero gap values in ROI calculation.
If gap is zero or negative, ROI may be zero or negative, which is likely not meaningful. Consider ensuring ROI is non-negative or handling non-positive gap values explicitly.
Suggested change
| # ROI scala con il gap da colmare | |
| roi = base_roi[component] * (gap / 30) | |
| return f"{int(roi)}% a 3 anni" | |
| # ROI scala con il gap da colmare | |
| if gap <= 0: | |
| return "Nessun ROI calcolabile: gap nullo o negativo" | |
| roi = base_roi[component] * (gap / 30) | |
| roi = max(0, roi) | |
| return f"{int(roi)}% a 3 anni" |
Comment on lines
+49
to
+55
| # Validazione base | ||
| if not answers: | ||
| return jsonify({'error': 'Nessuna risposta fornita'}), 400 | ||
|
|
||
| # Calcola GIST Score | ||
| calculator = GISTCalculator(company_name) | ||
| results = calculator.calculate_score(answers) |
There was a problem hiding this comment.
suggestion: Consider validating the structure of answers before calculation.
Malformed or incomplete answer data could cause errors or incorrect results. Adding validation will help ensure reliable calculations.
Suggested change
| # Validazione base | |
| if not answers: | |
| return jsonify({'error': 'Nessuna risposta fornita'}), 400 | |
| # Calcola GIST Score | |
| calculator = GISTCalculator(company_name) | |
| results = calculator.calculate_score(answers) | |
| # Validazione base | |
| if not answers: | |
| return jsonify({'error': 'Nessuna risposta fornita'}), 400 | |
| def validate_answers_structure(answers): | |
| # Esempio di validazione: answers deve essere un dict con almeno una chiave e valore numerico | |
| if not isinstance(answers, dict): | |
| return False | |
| if not answers: | |
| return False | |
| for key, value in answers.items(): | |
| if not isinstance(key, str): | |
| return False | |
| if not isinstance(value, (int, float)): | |
| return False | |
| return True | |
| if not validate_answers_structure(answers): | |
| return jsonify({'error': 'Struttura delle risposte non valida'}), 400 | |
| # Calcola GIST Score | |
| calculator = GISTCalculator(company_name) | |
| results = calculator.calculate_score(answers) |
| {% block extra_scripts %} | ||
| <script> | ||
| // Load results from sessionStorage | ||
| const results = JSON.parse(sessionStorage.getItem('gist_results')); |
There was a problem hiding this comment.
suggestion: Consider fallback for missing or corrupted sessionStorage data.
Instead of redirecting immediately, show an error message or offer a recovery option to improve user experience.
| if __name__ == '__main__': | ||
| app.run(debug=True) | ||
| # Development mode | ||
| app.run(debug=True, host='0.0.0.0', port=5000) |
There was a problem hiding this comment.
security (python.flask.security.audit.debug-enabled): Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables.
Source: opengrep
Comment on lines
+79
to
+87
| gist_score = 0 | ||
|
|
||
| for component, score in component_scores.items(): | ||
| if component in self.weights: | ||
| # Formula con esponente per rendimenti decrescenti | ||
| weighted_score = self.weights[component] * (score ** self.gamma) | ||
| gist_score += weighted_score | ||
|
|
||
| return gist_score |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Convert for loop into call to sum() (
sum-comprehension) - Inline variable that is only used once (
inline-variable) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
Suggested change
| gist_score = 0 | |
| for component, score in component_scores.items(): | |
| if component in self.weights: | |
| # Formula con esponente per rendimenti decrescenti | |
| weighted_score = self.weights[component] * (score ** self.gamma) | |
| gist_score += weighted_score | |
| return gist_score | |
| return sum( | |
| self.weights[component] * (score**self.gamma) | |
| for component, score in component_scores.items() | |
| if component in self.weights | |
| ) |
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.
Summary by Sourcery
Upgrade the GIST assessment web application to a full-featured interactive tool: expand the questionnaire to 36 calibrated questions, refactor the scoring engine for richer outputs (maturity levels, recommendations, benchmarks, ROI), and redesign the Flask front-end into modular templates with Tailwind CSS and Chart.js for a modern user experience.
New Features:
Enhancements:
Build:
Documentation: