[additive_functionals] Style guide review#263
Closed
github-actions[bot] wants to merge 1 commit intomainfrom
Closed
[additive_functionals] Style guide review#263github-actions[bot] wants to merge 1 commit intomainfrom
github-actions[bot] wants to merge 1 commit intomainfrom
Conversation
- code: 1 fixes - format: 9 fixes - writing: 28 fixes Rules addressed: - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - qe-writing-002: One-Sentence Paragraphs - ... and 28 more
✅ Deploy Preview for lustrous-melomakarona-3ee73e ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mmcky
added a commit
to QuantEcon/action-style-guide
that referenced
this pull request
Oct 1, 2025
CRITICAL FIX: The regex pattern for extracting corrected content from LLM responses was using non-greedy matching (.*?) which would stop at the first ``` encountered. This caused it to extract only partial content when the corrected lecture contained nested code blocks. Changes: - Updated parse_markdown_response() to handle nested code blocks correctly - Added fallback extraction logic for robustness - Updated test to validate nested code blocks are preserved - Removed misleading comment about "acceptable" partial parsing Impact: - Fixes issue where corrected content was truncated to just one line - Now properly extracts full corrected lecture content - Handles lectures with multiple code blocks (Python, Julia, etc.) See: QuantEcon/lecture-python-advanced.myst#263
Contributor
|
Close as test run used for diagnostics. |
mmcky
added a commit
to QuantEcon/action-style-guide
that referenced
this pull request
Oct 1, 2025
…tent MAJOR IMPROVEMENT: Complete overhaul of how fixes are applied to lectures. Problem: - LLMs were generating entire corrected lecture content - This was unreliable: truncation, added commentary, deleted content - PR #263 showed lecture being deleted except one line Solution: - LLM now only identifies violations with exact current_text and suggested_fix - New fix_applier.py module applies fixes programmatically via text replacement - Preserves all original content, only applies targeted fixes - No LLM-generated notes or commentary in the output Changes: 1. Created style_checker/fix_applier.py: - apply_fixes(): Programmatic text replacement - validate_fix_quality(): Check for commentary, missing fields - Detailed warnings for fixes that can't be applied 2. Updated reviewer.py: - Modified all provider prompts to NOT generate corrected content - Added apply_fixes() call in review_lecture() - Updated review_in_chunks() to apply fixes to original (not sequential) - Made corrected_content field optional in parser 3. Benefits: - ✅ Keeps original lecture intact - ✅ Only applies targeted fixes - ✅ No LLM commentary in output - ✅ Better error handling - ✅ Validation warnings for bad fixes - ✅ Handles cases where exact text can't be found Impact: - PRs will now show precise, targeted changes - No more deleted content - No more LLM notes in the lecture - More reliable and predictable fixes See: QuantEcon/lecture-python-advanced.myst#263
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.
📋 Style Guide Review: additive_functionals
This PR addresses style guide compliance issues found in the
additive_functionalslecture.📊 Summary
🎯 Issues by Priority
📝 Detailed Changes
Code (1 issues)
ϕ_1, ϕ_2, ϕ_3, ϕ_4 = 0.5, -0.2, 0, 0.5 σ = 0.01 ν = 0.01 # Growth rate...φ_1, φ_2, φ_3, φ_4 = 0.5, -0.2, 0, 0.5 σ = 0.01 ν = 0.01 # Growth rate...Format (9 issues)
qe-format-001 - Definitions
*multiplicative functionals...`
*multiplicative functionals...`
qe-format-001 - Definitions
qe-format-001 - Definitions
We construct our additive functional from two pieces, the first of which is a **first-order vector ...We construct our additive functional from two pieces, the first of which is a **first-order vector a...qe-format-002 - Emphasis
The nonstationary random process $\{y_t\}_{t=0}^\infty$ displays systematic but random *arithmetic g...The nonstationary random process $\{y_t\}_{t=0}^\infty$ displays systematic but random *arithmetic g...qe-format-002 - Emphasis
We have chosen to simulate many paths, all starting from the *same* non-random initial conditions $x...We have chosen to simulate many paths, all starting from the *same* non-random initial conditions $x...qe-format-001 - Definitions
As mentioned above, the process $\{M_t\}$ is called a **multiplicative functional**....As mentioned above, the process $\{M_t\}$ is called a **multiplicative functional**....qe-format-001 - Definitions
The second is a **peculiar property** noted and proved by Hansen and Sargent {cite}Hans_Sarg_book....The second is a **peculiar property** noted and proved by Hansen and Sargent {cite}Hans_Sarg_book....qe-format-001 - Definitions
[This lecture](https://python.quantecon.org/likelihood_ratio_process.html) studies **likelihood proc...[This lecture](https://python.quantecon.org/likelihood_ratio_process.html) studies **likelihood proc...qe-format-003 - Jupyter Book Theme
additive_functionalswhich is not consistent with Jupyter Book best practices. Labels should use hyphens for better URL formatting and consistency with Jupyter Book conventions.(additive_functionals)=...(additive-functionals)=...Writing (28 issues)
qe-writing-002 - One-Sentence Paragraphs
Many economic time series display persistent growth that prevents them from being asymptotically st...Many economic time series display persistent growth that prevents them from being asymptotically sta...qe-writing-002 - One-Sentence Paragraphs
But there are good ways to model time series that have persistent growth that still enable statisti...But there are good ways to model time series that have persistent growth that still enable statistic...qe-writing-002 - One-Sentence Paragraphs
If a process${y_t}$ is an additive functio...`
If a process${y_t}$ is an additive functio...`
{cite}Hansen_2012_Eca` describes a general class of additive functionals.This lecture focuses on ...`
{cite}Hansen_2012_Eca` describes a general class of additive functionals.This lecture focuses on ...`
qe-writing-002 - One-Sentence Paragraphs
Our special additive functional displays interesting time series behavior while also being easy to c...Our special additive functional displays interesting time series behavior while also being easy to c...qe-writing-002 - One-Sentence Paragraphs
initial condition for
The nonstati...`
The nonstati...`
qe-writing-002 - One-Sentence Paragraphs
A convenient way to represent our additive functional is to use a [linear state space system](https:...A convenient way to represent our additive functional is to use a [linear state space system](https:...qe-writing-002 - One-Sentence Paragraphs
To study it, we could map it into an instance of [LinearStateSpace](https://github.com/QuantEcon/Qua...To study it, we could map it into an instance of [LinearStateSpace](https://github.com/QuantEcon/Qua...qe-writing-002 - One-Sentence Paragraphs
(addfunc_eg1)=
In doing so we'll assume that $z_{t+1...`
(addfunc_eg1)=
In doing so we'll assume that $z_{t+1...`
$$
\phi(z) = ( 1 - \phi_1 z - \phi_2 z^2 - \phi_3 z^3 - ...`
$$
\phi(z) = ( 1 - \phi_1 z - \phi_2 z^2 - \phi_3 z^3 - \p...`
While {eq}ftafis not a first order system like {eq}old1_additive_functionals, we know that it c...While {eq}ftafis not a first order system like {eq}old1_additive_functionals, we know that it c...This system also constructs the compon...`
This system also constructs the compon...`
When we plot multiple realizations of a component in the 2nd, 3rd, and 4th panels, we also plot the ...When we plot multiple realizations of a component in the 2nd, 3rd, and 4th panels, we also plot the ...the purple one for the martinga...`
the purple one for the martinga...`
As mentioned above, the process ...`
As mentioned above, the process ...`
An instance of classAMF_LSS_VAR({ref}above <amf_lss>) includes this associated multiplicative...An instance of classAMF_LSS_VAR({ref}above <amf_lss>) includes this associated multiplicative ...As before, when we plotted multiple realizations of a component in the 2nd, 3rd, and 4th panels, we ...As before, when we plotted multiple realizations of a component in the 2nd, 3rd, and 4th panels, we ...Hansen and Sargent {cite}Hans_Sarg_book(ch. 8) describe the following two properties of the mart...Hansen and Sargent {cite}Hans_Sarg_book(ch. 8) describe the following two properties of the marti...It remains con...`
It remains con...`
Let's drill down and study probability distribution of the multiplicative martingale $\{\widetilde ...Let's drill down and study probability distribution of the multiplicative martingale $\{\widetilde M...It follows that $\log {\widetilde M}_t \sim {\mathcal N} ( -\frac...`
It follows that $\log {\widetilde M}_t \sim {\mathcal N} ( -\frac{...`
...`
...`
We'll do this ...`
We'll do this ...`
The heavy lifting is done inside theAMF_LSS_VAR` class.The following code adds some simple funct...`
The heavy lifting is done inside theAMF_LSS_VAR` class.The following code adds some simple funct...`
Now that we have these functions in our toolkit, let's apply them to run some simulations....Now that we have these functions in our toolkit, let's apply them to run some simulations....Let's plot the probability density functions for $\log {\widetilde M}_t$ for $t=100, 500, 1000, 1000...Let's plot the probability density functions for $\log {\widetilde M}_t$ for $t=100, 500, 1000, 1000...These probability density functions help us understand mechanics underlying the **peculiar property...These probability density functions help us understand mechanics underlying the **peculiar property*...[This lecture](https://python.quantecon.org/likelihood_ratio_process.html) studies **likelihood proc...[This lecture](https://python.quantecon.org/likelihood_ratio_process.html) studies **likelihood proc...📌 Summary
Found 38 issues across 4 rule checks
🤖 This PR was automatically generated by the QuantEcon Style Guide Checker
📚 Review the Style Guide Documentation for more details