-
Notifications
You must be signed in to change notification settings - Fork 62
Projector in terms #2078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Projector in terms #2078
Conversation
Plan to add Projector constructors to exp_term, pat_term, typ_term to enable round-tripping of projector data through segment -> term -> segment. Key design decisions: - Create src/language/ProjectorKind.re for shared Kind.t enum - Construct Projector term directly in MakeTerm tile_kids - Use RemoveParens step kind (with note about future RemoveProjector) - All projector kinds neutral at term level Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add Projector constructors to exp_term, pat_term, and typ_term to preserve projector metadata (kind, model) through the segment → term → segment cycle. Key changes: - Create ProjectorKind.re to break dependency cycle between Grammar and ProjectorCore - Add Projector(projector_data, inner) constructors to Grammar.re - Update MakeTerm.re to construct Projector terms directly in tile_kids - Update ExpToSegment.re to serialize Projector terms back to Piece.Projector - Add Projector handling to SecondaryCollection for whitespace preservation - Remove trim_secondary hack from Triggers.re (no longer needed) - Add pass-through cases in Statics, Dynamics, Coverage, etc. - Add projector round-trip tests including inner spacing preservation Co-Authored-By: Claude Opus 4.5 <[email protected]>
… projector-in-terms
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #2078 +/- ##
==========================================
- Coverage 50.48% 50.36% -0.12%
==========================================
Files 229 230 +1
Lines 25202 25368 +166
==========================================
+ Hits 12722 12777 +55
- Misses 12480 12591 +111
🚀 New features to boost your workflow:
|
cyrus-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- cursor inspector should say what the projector name is rather than just Projector
|
@cyrus- projector kind now exposed in CI |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
|
can merge once conflict is resolved |
|
@copilot can you fix the merge conflict |
TL;DR: Continuing #2077. Mostly pure refactor; some minor differences to cursor inspector etc experience around projectors. Adds projectors as a wrapping forms in terms (previously projectors only lived in segment), increasing segment<->term roundtripability.
Projector Term Constructor Plan
Goal
Add
Projectorconstructors to the term grammar (exp_term,pat_term,typ_term) to enable round-tripping of projector data through the segment → term → segment cycle.Currently, when segments containing projectors are parsed via MakeTerm, the projector metadata (
kind,model) is discarded—only the inner syntax is preserved. This prevents faithful round-tripping.Current State
Segment-Level Projectors
In
ProjectorCore.re:In
Base.re:Projector Syntax Structure
Key invariant: The
syntaxfield is always a parenthesizedPiece:Segment.parenthesize(seg)inProjectorPerform.initPiece.unparenthesize(syntax)extracts this payloadCurrent MakeTerm Handling
In
tile_kids:The
PROJ_WRAPhack in exp/pat/typ pattern matching:This discards all projector metadata.
Proposed Design
1. Projector Data Type
Define in
Grammar.re(or a shared location):Dependency resolution: Create
src/language/ProjectorKind.recontaining just theKind.tenum. BothGrammar.reandProjectorCore.rewill import from this shared location.Current Kind.t values (from ProjectorCore.re):
Also copy over the
nameandof_namefunctions, and any other Kind-related utilities that Grammar might need.2. Term Constructors
Add to
exp_term:Add to
pat_term:Add to
typ_term:TPat: Omit for now given its minimal scope.
3. ID Handling
The projector's ID uses the standard annotation system:
Projector(data, inner)term has its own annotation containingidsrep_idextracts the primary IDParens(inner)worksImplementation Changes Required
MakeTerm.re
Key change in
tile_kids: Construct the Projector term directly when processing projector pieces:PROJ_WRAP pattern matching stays unchanged:
Now
body.termis alreadyProjector(...), so this just passes it through.ExpToSegment.re
Add
Projectorcases that serialize back toPiece.Projector:Grammar.re: map_*_annotation Functions
Add Projector cases (straightforward pass-through):
TermBase.re: map_term Functions
Statics.re
Follow the Parens pattern—bidirectional type propagation. Looking at actual Parens handling:
Expressions (uexp_to_info_map, ~line 373):
So for Projector:
Patterns (upat_to_info_map, ~line 1867):
Types (utyp_to_info_map, ~line 1958):
All are simple pass-through—the Projector wrapper is transparent to typechecking.
Dynamics / Transition.re
Follow the Parens pattern—
RemoveParensstep removes the wrapper:Pattern Matching (PatternMatch.re)
Substitution.re
EvalCtx.re
Add
Projectorto the evaluation context type:And the compose function.
Coverage.re
Treat as degenerate (like Parens):
Exp.re, Pat.re, Typ.re Utility Functions
Many functions that recurse through terms need Projector cases:
is_fun,is_var,get_var, etc.Elaborator.re
Form.re
Consider whether a distinct
ProjectorExp/ProjectorPat/ProjectorTypform is needed, or if thePROJ_WRAPapproach can be refined.Abbreviate.re
Add cost for projector (similar to Parens):
ProofHacks.re
Handle in exp_to_pat, pat_to_exp, and inductive hypothesis extraction.
Grammar.re: Factory Module
Grammar.re has a
Factorymodule with helper constructors likeExp.parens. Add corresponding helpers:Similarly for Pat and Typ.
ExpToSegment.re: Precedence Functions
The
external_precedenceandinternal_precedencefunctions handle Parens by returningPrecedence.max(highest precedence, no wrapping needed). Projector should do the same:Equality.re
Term equality checking may need Projector cases. Check if there's explicit pattern matching on term constructors.
Files Requiring Changes (Comprehensive List)
Based on Parens handling analysis:
Core Grammar & Types:
Grammar.re- Type definitionsTermBase.re- map_term functionsExp.re,Pat.re,Typ.re- Utility functionsParsing:
MakeTerm.re- Term constructionForm.re- Form definitions (if needed)Pretty Printing:
ExpToSegment.re- SerializationStatics:
Statics.re- Type checkingElaborator.re- ElaborationCoverage.re- Coverage checkingDynamics:
Transition.re- Evaluation stepsPatternMatch.re- Pattern matchingSubstitution.re- SubstitutionEvalCtx.re- Evaluation contextsEvaluatorStep.re- StepperDHExp.re- ty_substProof:
ProofHacks.re- Proof utilitiesDisplay:
Abbreviate.re- AbbreviationProjectors:
CardProj.re- Card projector (if it pattern matches on terms)Resolved Design Questions
1. MakeTerm Threading (RESOLVED)
Problem: The
PROJ_WRAPpattern matching loses projector metadata (kind, model) because by that point we only have tokens and kids.Solution: Construct Projector term directly in
tile_kidswhere we have all the metadata. The PROJ_WRAP pattern match then just passes through the already-constructed term.See "MakeTerm.re" section above for implementation details.
2. Kind.t Dependency (RESOLVED)
Create
src/language/ProjectorKind.rewith just the enum. Both Grammar.re and ProjectorCore.re import from there.3. RemoveProjector vs RemoveParens (RESOLVED)
Use
RemoveParensfor now. Add a comment noting we may wantRemoveProjectorlater for stepper clarity.Remaining Considerations
Round-Trip Test Structure
The tests in
Test_ExpToSegment.recurrently explicitly exclude projectors (noted as "out of scope" around line 652). With projectors in terms, we can add:^^fold 1Projector ID vs Inner Term ID
When a projector wraps a term:
pr.iddiffers from inner piece IDsBuild Configuration
Creating
src/language/ProjectorKind.remay require updatingsrc/language/duneto include the new module. Check the dune file structure and add the module if needed.Decision Points
Kind.t dependency approach: Create
src/language/ProjectorKind.rewith just the enum. Both Grammar.re and ProjectorCore.re will use this shared definition.MakeTerm threading approach: Construct Projector term directly in
tile_kids.RemoveProjector vs RemoveParens step kind: Use
RemoveParensfor now. Add a comment at the implementation noting we may want a distinctRemoveProjectorstep kind later for stepper clarity.All projector kinds neutral at term level: Yes. All projector kinds behave the same at the term level—just wrap. Projector-specific behavior stays at segment/UI level.
Implementation Notes
Dynamics Flag on Projectors
Projectors have a
dynamics: boolflag that was used to determine whether to collect dynamics / create sample targets in the evaluator. The only projector that used this (Probe) is now a refractor, so this may be dormant.Verified: The dynamics flag is NOT checked in MakeTerm—it's only referenced in the UI layer (RefractorView.re, ProjectorView.re) and ProjectorBase.re definition. This is currently a UI-level concern, not a term-level concern. If we later need projectors that require dynamics collection at the term level, that machinery would need to be introduced. For now, this doesn't affect implementation.
Secondary Handling for Projectors
Projectors should act like any other term for secondary (whitespace/comments) handling. In
tile_kids, when constructing the Projector term annotation, useget_secondary([id])whereidis the projector's ID.TODO: Verify during implementation that projector IDs are present in the
secondary_map. If the projector piece's secondary isn't being collected, we may need to ensure it's added to the map, or fall back to empty secondary([], []).