fix(symbolicai): promote 6 BETA notebooks to PRODUCTION (Epic #1947)#1952
fix(symbolicai): promote 6 BETA notebooks to PRODUCTION (Epic #1947)#1952jsboige wants to merge 1 commit into
Conversation
Notebook PR Validation: PASS
Checks: H.1 (no errors), H.3 (execution_count), C.1 (no banned patterns) |
jsboige
left a comment
There was a problem hiding this comment.
[NanoClaw]
Review Summary
PR delivers real content work (definition condensation, exercise stubs, catalog reconciliation) but has a few issues that should be addressed before merge.
1. Title vs Reality: No BETA-to-PRODUCTION promotion in this diff
The PR title says "promote 6 BETA notebooks to PRODUCTION" but the catalog diff contains zero maturity changes. All 4 Argument_Analysis Agentic notebooks remain BETA in COURSE_CATALOG.generated.json. The 131/-131 lines are exclusively:
cells_with_outputs/cells_without_outputscount adjustments (many notebooks cleaned up empty output cells)last_validationdate bumps to2026-05-31issue_pr_associeeupdates (e.g.#1901->#1931,#1769->#1931,#1871->#1931)
Recommendation: Either update the PR title to reflect the actual scope (definition condensation + catalog reconciliation), or add the missing maturity promotion changes if that was the intent. Epic #1947 context would help clarify.
2. Papermill metadata leak in Agentic-0-init.ipynb [WARNING]
The notebook metadata block contains a full papermill execution record:
"papermill": {
"input_path": "MyIA.AI.Notebooks/SymbolicAI/.../Argument_Analysis_Agentic-0-init.ipynb",
"output_path": "C:/Users/jsboi/AppData/Local/Temp/aa0_exec.ipynb",
"start_time": "2026-05-31T19:06:44.275528+00:00",
"duration": 65.163106,
"version": "2.7.0"
}This leaks a local Windows temp path (C:/Users/jsboi/AppData/...). Papermill metadata is also present in Agentic-1, Agentic-2, Planners-3, and Tweety-6 (14-51 refs each) though those don't have path leaks.
Recommendation: Strip papermill metadata before committing. A nbstripout pre-commit hook or a cleanup pass would prevent this.
3. Notebook content changes: Generally good
Agentic-0-init (+756/-306): Major expansion is real new content -- not verbose outputs. 27 cells total, 10 code cells all with valid non-null execution_count and legitimate outputs. The expansion adds initialization code, configuration, and definition cells. No fake outputs detected.
Agentic-1 (-117), Agentic-2 (-253), Agentic-3 (-20): These are definition cell condensation, not content deletion. The markdown introductions are compressed from detailed prose into shorter summaries while preserving all technical details (method signatures, prompt contents, architecture tables). The previously empty outputs: [] are now replaced with real print() statements producing confirmation messages. This is consistent with the #1924 review that flagged Agentic-2's cleared outputs.
Planners-3 (+20/-18): Exercise cells changed from outputs: [] to print("Exercice X a completer...") stubs. Clean pedagogical pattern -- students see the placeholder, not a blank cell.
Tweety-6 (+10/-40): Same pattern -- exercise cell compressed, empty outputs replaced with stub print.
4. Security: No secrets, one minor note
api_key=api_keyin Agentic-0 is a variable reference (function parameter), not a hardcoded credential. OK.tokens>=2.1.0is a pip dependency string. OK.- No other secret patterns detected.
- The only security concern is the papermill path leak noted in #2.
5. Catalog: Cells-with-outputs reductions make sense
The consistent pattern of decreasing cells_with_outputs and cells_without_outputs across many notebooks (including non-SymbolicAI ones) is consistent with the content changes: empty outputs are being replaced with stub prints (net reduction in cells_without_outputs to 0 for many entries).
Action Items
| Priority | Item | File |
|---|---|---|
| HIGH | Fix PR title or add missing BETA->PROD changes | PR title / COURSE_CATALOG |
| MEDIUM | Strip papermill metadata (local path leak) | Agentic-0-init.ipynb |
| LOW | Consider nbstripout pre-commit hook for the repo |
Repo config |
clusterManager-Myia
left a comment
There was a problem hiding this comment.
[Hermes] — COMMENT_WITH_CONCERNS
Security scan: CLEAN (OPENAI_API_KEY refs in removed lines are error messages about missing config, not actual key values).
Correction factuelle: Le review NanoClaw indique "zero maturity changes" — c'est incorrect. Le diff contient bien 6 promotions BETA→PRODUCTION dans COURSE_CATALOG (PRODUCTION 88→94, BETA 11→5 dans README.md). Le titre de la PR est exact.
Concern:
- Papermill metadata leak (confirme NanoClaw) : Agentic-0-init.ipynb contient
"output_path": "C:/Users/jsboi/AppData/Local/Temp/aa0_exec.ipynb". A nettoyer avant merge — nbstripout ou nettoyage manuel.
…+ catalog regen Promote AA-0/1/2/3, Planners-3, Tweety-6 BETA->PRODUCTION. Strip local Papermill path from AA-0-init metadata. Catalog regen --git-tracked-only + expand markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6c62430 to
d31ba0a
Compare
|
Superseded by #2020 (clean rebase on latest main + catalog regen). All content preserved via cherry-pick. |
Summary
Promote 6 SymbolicAI BETA notebooks to PRODUCTION maturity (Epic #1947).
Changes
Argument Analysis (4 notebooks):
Planners-3-State-Space:
# TODO etudiantmarkers to 2 exercise cells (convention docs(rules): code cells emit informative output (anti false-positive maturity) #1946)Tweety-6-Structured-Argumentation:
# TODO etudiantmarker + print to 1 exercise cellMaturity Verification
All 6 notebooks now classify as PRODUCTION per
generate_catalog.py:Execution Evidence
Closes #1947