Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
391 changes: 391 additions & 0 deletions assessment/issue_23_implementation_plan.md

Large diffs are not rendered by default.

86 changes: 86 additions & 0 deletions assessment/issue_selection_recommendation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
name: Issue Selection Recommendation
overview: Analysis of all 19 open issues against the user's criteria (fix existing > new features, no dependency blockers, achievable in a few hours) to recommend the best issue for this take-home assessment.
todos:
- id: update-prompt-log
content: Update prompt-history.md with this prompt
status: pending
- id: confirm-issue
content: "Confirm issue #23 selection with user before starting implementation"
status: pending
isProject: false
---

# Issue Selection Recommendation

## Issue landscape (19 open)

### Already taken (PRs exist -- avoid these)

- **#20** "Add negative-path REST tests for note retrieval and import validation" -- open PR #27
- **#21** "Add graph API tests for conflict and not-found mutation paths" -- open PR #28
- **#29** "Apple Calendar settings UX improvements" -- open PR #32

### Too large / external dependencies (eliminate)

- **#43** JVM scheduler backed by Timefold -- requires JVM ecosystem, weeks of work
- **#40** Open Banking integration -- requires third-party API provider, massive scope
- **#41** Embed notes into vector store for MCP semantic search -- requires OPENAI_API_KEY, Milvus, complex pipeline work
- **#47** MCP for conversational interaction with finances -- depends on #40 (Open Banking not built yet)
- **#46** MCP for conversational interaction with calendar -- large feature, MCP + calendar
- **#37** Apple Calendar full connect-to-sync flow -- large multi-service integration
- **#38** Completed nodes orbit as stars -- heavy 3D WebGL/Three.js work in a 176K-char file
- **#39** Notes render as parentless stars -- depends on #38

### Feasible candidates (ranked by your criteria)

#### Tier 1 -- Fixes existing mismatches (highest priority)

**#23: Stop advertising unsupported schedule mutations in SDK and MCP**

- **Why it's the best fit:**
- This is a **contract-cleanup / bug-fix**, not a new feature. The service layer already rejects `replaceSchedule` and `deleteSchedule` with `ConflictError`, but the SDK, hooks, and MCP tool registry still advertise them. This is a real misleading API surface.
- **Zero external dependencies.** No Docker needed to develop -- pure code changes + existing unit tests.
- Touches exactly 4 files: `client.js`, `useActivityGraph.js`, `tool_registry.py`, and their tests.
- Validation is straightforward: `npm test` (SDK) + `python -m unittest` (Python).
- Demonstrates **systems thinking** (understanding the REST/SDK/MCP transport boundary) and **engineering judgment** (intentional surface-area reduction).
- ~2-3 hours of focused work.

**#26: Make the web app show when running on mock fallback data**

- Fixes a **product-quality gap** (confusing silent fallback).
- Frontend-only, no external deps.
- But: the `App.jsx` file is 176K characters -- navigating it is harder and riskier. Less testable with automated checks.

#### Tier 2 -- Test coverage (valuable but less "pressing")

- **#22** Add unit tests for MCP JSON-RPC error handling -- good scope, Python only, ~2-3 hours
- **#24** Expand Move37Client tests -- SDK only, ~2 hours
- **#25** Add useNotes hook tests -- SDK only, ~1-2 hours

These are well-scoped but are purely additive test coverage, not fixing a mismatch in existing functionality.

#### Tier 3 -- Small UI features (feasible but lower priority)

- **#34** Inline URL import input -- small frontend change
- **#35** Direct 3D/linear layout transitions -- frontend, tricky 3D work
- **#36** Auto-fit viewport on linear mode switch -- frontend, depends on understanding 3D layout

## Recommendation

**Issue #23: "Stop advertising unsupported schedule mutations in SDK and MCP"**

This is the strongest choice because:

1. **Fixes existing broken contract** -- the SDK and MCP surfaces promise operations that always fail. This is the most "pressing" issue for anyone consuming the API today.
2. **No dependency blockers** -- no Docker, no OPENAI_API_KEY, no external services needed. Pure code + tests.
3. **Right scope for a take-home** -- ~2-3 hours, touches Python (MCP tool registry) + JavaScript (SDK client + React hooks) + tests on both sides, demonstrating cross-stack competence.
4. **Strong assessment signal** -- shows you understand the architecture (service layer vs. transport surfaces), can make a safe contract reduction, and can validate across both test suites.

### Files you would change

- [src/move37/sdk/node/src/client.js](src/move37/sdk/node/src/client.js) -- remove `replaceSchedule()` and `deleteSchedule()`
- [src/move37/sdk/node/src/hooks/useActivityGraph.js](src/move37/sdk/node/src/hooks/useActivityGraph.js) -- remove from returned hook API
- [src/move37/api/tool_registry.py](src/move37/api/tool_registry.py) -- remove `activity.schedule.replace` and `schedule.delete` tool definitions
- SDK and Python tests to cover the removals

49 changes: 49 additions & 0 deletions assessment/prompt-history.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Prompt History

Chronological record of key prompts used with the coding assistant during this assessment.

---

**Prompt 0:** Read through the repository (locally, /Users/wallace5/Move37) to better understand what this codebase does.

**Prompt 1:** Read the contributing-docs. What do I need to do first to set up before contributing?

**Prompt 2:** Start this setup (local environment setup).

**Prompt 3:** First we need to fork the repo.

**Prompt 4:** Clone my fork to my local, or overwrite the existing clone to point to the fork (no changes have been made yet).

**Prompt 5:** What do I need to include in a PR for this assessment?

**Prompt 6:** Create an .md file to record prompts. Every prompt given from now on should be recorded there.

**Prompt 7:** Review all open issues. Recommend a task to work on using the following criteria: (1) Which issues are most pressing given the current state of the repository? Prioritise fixing existing functionality over adding new features. (2) Which issues do not require dependencies that would be blockers? (3) Which of these is appropriate for this take-home task?

**Prompt 8:** Issue #23 seems like a good choice. Open a new branch for this project. Open a PR but do not populate it yet.

**Prompt 9:** Read through the issue in detail and create a plan to execute this change. The plan should be written to the PR. Consider which files will need to be changed, how and any anticipated pitfalls. This should detail an ideal solution, it does not all need to be completed in this 2-3 hour session. The PR goals should be separated into core, additional and stretch.

**Prompt 10:** What would make this plan better/more robust?

**Prompt 11:** Incorporate these changes. The commit strategy should be 3 commits, one for core, one for additional and one for stretch.

**Prompt 12:** The validation checklist needs to be more thorough. I want a list of checkboxes for core, additional and stretch that can be iterated through after each section is completed.

**Prompt 13:** Begin executing the core requirements. After you have done C1-4, run all tests laid out in the validation checklist. Make a note of those that fail for my reference and iterate until all tests have passed. Do not commit any code without checking with me first.

**Prompt 14:** If all tests are passing and the validation checklist has been met, commit the changes and then proceed with the additional section.

**Prompt 15:** Review what has been done. Detail what changes have been made, their impact on the repo's functionality and how I can see this for myself.

**Prompt 16:** Implement the stretch goals and the validation checklist associated with it.

**Prompt 17:** Are there any more challenging stretch goals that we can add to this plan?

**Prompt 18:** Add S4, S5, S6 to the plan as stretch 2. Come up with a validation checklist to ensure these are correct and do not break anything.

**Prompt 19:** Integrate this into the original plan .md.

**Prompt 20:** Implement stretch 2 goals. Ensure all validation checklist items are met.

**Prompt 21:** Create the PR. Move the two plans and the prompt history to a tmp/ folder and reference these in the PR. Read the PR instructions in detail to ensure that you meet all of the criteria.
4 changes: 4 additions & 0 deletions fern/pages/sdks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ Generated output lands in:
## Existing SDK code

The repository still contains a hand-written Node client in `src/move37/sdk/node`. Keep that in mind while migrating toward generated SDKs. During the transition, generated SDK output should be evaluated before replacing the current package surface.

## Schedule edge mutations

Schedule edges in Move37 are derived automatically from activity `startDate` values and the deterministic planner. The hand-written Node SDK intentionally omits `replaceSchedule` and `deleteSchedule` — use `updateActivity` with a new `startDate` or the `/v1/scheduling/replan` endpoint to change scheduling. The REST schedule mutation endpoints exist for backward compatibility but always return `409 Conflict`.
20 changes: 16 additions & 4 deletions src/move37/api/routers/rest/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,20 @@ def activity_replace_dependencies(
return ActivityGraphOutput(**result)


@router.put("/activities/{activity_id}/schedule", response_model=ActivityGraphOutput)
@router.put(
"/activities/{activity_id}/schedule",
response_model=ActivityGraphOutput,
deprecated=True,
description="Unsupported. Schedule edges are derived from startDate. Always returns 409.",
responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}},
)
def activity_replace_schedule(
activity_id: str,
payload: ReplaceScheduleInput,
subject: Annotated[str, Depends(get_current_subject)],
services: Annotated[ServiceContainer, Depends(get_service_container)],
) -> ActivityGraphOutput:
"""Replace a node's schedule relations."""
"""Replace a node's schedule relations (unsupported -- always returns 409)."""

try:
result = services.activity_graph_service.replace_schedule(
Expand Down Expand Up @@ -219,14 +225,20 @@ def dependency_delete(
return ActivityGraphOutput(**result)


@router.delete("/schedules/{earlier_id}/{later_id}", response_model=ActivityGraphOutput)
@router.delete(
"/schedules/{earlier_id}/{later_id}",
response_model=ActivityGraphOutput,
deprecated=True,
description="Unsupported. Schedule edges are derived from startDate. Always returns 409.",
responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}},
)
def schedule_delete(
earlier_id: str,
later_id: str,
subject: Annotated[str, Depends(get_current_subject)],
services: Annotated[ServiceContainer, Depends(get_service_container)],
) -> ActivityGraphOutput:
"""Delete a schedule edge."""
"""Delete a schedule edge (unsupported -- always returns 409)."""

try:
result = services.activity_graph_service.delete_schedule(subject, earlier_id, later_id)
Expand Down
15 changes: 0 additions & 15 deletions src/move37/api/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,6 @@ class ReplaceActivityDependenciesInput(ReplaceDependenciesInput):
activityId: str


class ReplaceActivityScheduleInput(ReplaceScheduleInput):
"""Activity schedule replacement tool payload."""

activityId: str


class DependencyEdgeInput(BaseModel):
"""Dependency edge payload."""

Expand All @@ -545,14 +539,5 @@ class DependencyEdgeInput(BaseModel):
childId: str


class ScheduleEdgeInput(BaseModel):
"""Schedule edge payload."""

model_config = ConfigDict(extra="forbid")

earlierId: str
laterId: str


NoteCreateResponse.model_rebuild()
NoteImportResponse.model_rebuild()
28 changes: 0 additions & 28 deletions src/move37/api/tool_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,7 @@ def __init__(self, services: ServiceContainer) -> None:
"Replace an activity's dependencies.",
schemas.ReplaceActivityDependenciesInput,
),
ToolDefinition(
"activity.schedule.replace",
"Replace derived schedule edges for an activity.",
schemas.ReplaceActivityScheduleInput,
),
ToolDefinition("dependency.delete", "Delete a dependency edge.", schemas.DependencyEdgeInput),
ToolDefinition("schedule.delete", "Delete a schedule edge.", schemas.ScheduleEdgeInput),
ToolDefinition("note.create", "Create a note and linked graph node.", schemas.NotePayload),
ToolDefinition("note.update", "Update a note and linked graph node.", schemas.UpdateNoteInput),
ToolDefinition("note.get", "Fetch a note.", schemas.NoteIdInput),
Expand All @@ -75,9 +69,7 @@ def __init__(self, services: ServiceContainer) -> None:
"activity.fork": self._handle_activity_fork,
"activity.delete": self._handle_activity_delete,
"activity.dependencies.replace": self._handle_activity_dependencies_replace,
"activity.schedule.replace": self._handle_activity_schedule_replace,
"dependency.delete": self._handle_dependency_delete,
"schedule.delete": self._handle_schedule_delete,
"note.create": self._handle_note_create,
"note.update": self._handle_note_update,
"note.get": self._handle_note_get,
Expand Down Expand Up @@ -193,16 +185,6 @@ def _handle_activity_dependencies_replace(self, subject: str, arguments: dict[st
)
).model_dump()

def _handle_activity_schedule_replace(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]:
payload = schemas.ReplaceActivityScheduleInput.model_validate(arguments)
return schemas.ActivityGraphOutput(
**self._services.activity_graph_service.replace_schedule(
subject,
payload.activityId,
[],
)
).model_dump()

def _handle_dependency_delete(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]:
payload = schemas.DependencyEdgeInput.model_validate(arguments)
return schemas.ActivityGraphOutput(
Expand All @@ -213,16 +195,6 @@ def _handle_dependency_delete(self, subject: str, arguments: dict[str, Any]) ->
)
).model_dump()

def _handle_schedule_delete(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]:
payload = schemas.ScheduleEdgeInput.model_validate(arguments)
return schemas.ActivityGraphOutput(
**self._services.activity_graph_service.delete_schedule(
subject,
payload.earlierId,
payload.laterId,
)
).model_dump()

def _handle_note_create(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]:
payload = schemas.NotePayload.model_validate(arguments)
response = self._services.note_service.create_note(subject, title=payload.title, body=payload.body)
Expand Down
15 changes: 0 additions & 15 deletions src/move37/sdk/node/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,6 @@ export class Move37Client {
});
}

replaceSchedule(activityId, peers) {
this.logOperation("replaceSchedule", { activityId, peers });
return this.request("PUT", `/v1/activities/${encodeURIComponent(activityId)}/schedule`, {
body: { peers },
});
}

deleteDependency(parentId, childId) {
this.logOperation("deleteDependency", { parentId, childId });
return this.request(
Expand All @@ -177,14 +170,6 @@ export class Move37Client {
);
}

deleteSchedule(earlierId, laterId) {
this.logOperation("deleteSchedule", { earlierId, laterId });
return this.request(
"DELETE",
`/v1/schedules/${encodeURIComponent(earlierId)}/${encodeURIComponent(laterId)}`,
);
}

async request(method, path, opts = {}) {
const query = toQueryString(opts.query);
const url = `${this.baseUrl}${path}${query ? `?${query}` : ""}`;
Expand Down
10 changes: 10 additions & 0 deletions src/move37/sdk/node/src/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,16 @@ describe("Move37Client", () => {
);
});

it("does not expose replaceSchedule", () => {
const client = new Move37Client({ baseUrl: "", fetchImpl: vi.fn() });
expect(client.replaceSchedule).toBeUndefined();
});

it("does not expose deleteSchedule", () => {
const client = new Move37Client({ baseUrl: "", fetchImpl: vi.fn() });
expect(client.deleteSchedule).toBeUndefined();
});

it("posts Apple Calendar connect payloads as JSON", async () => {
const fetchImpl = vi.fn(async () => ({
ok: true,
Expand Down
4 changes: 0 additions & 4 deletions src/move37/sdk/node/src/hooks/useActivityGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,8 @@ export function useActivityGraph(options) {
runStructuralMutation(() => client.deleteActivity(activityId, deleteTree)),
replaceDependencies: (activityId, parentIds) =>
runStructuralMutation(() => client.replaceDependencies(activityId, parentIds)),
replaceSchedule: (activityId, peers) =>
runStructuralMutation(() => client.replaceSchedule(activityId, peers)),
deleteDependency: (parentId, childId) =>
runStructuralMutation(() => client.deleteDependency(parentId, childId)),
deleteSchedule: (earlierId, laterId) =>
runStructuralMutation(() => client.deleteSchedule(earlierId, laterId)),
};
}

Expand Down
20 changes: 20 additions & 0 deletions src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,26 @@ describe("useActivityGraph", () => {
expect(result.current.graph.nodes).toHaveLength(1);
});

it("does not expose replaceSchedule or deleteSchedule", async () => {
const fetchImpl = vi.fn(async () =>
createJsonResponse({
graphId: 1,
version: 1,
nodes: [],
dependencies: [],
schedules: [],
}),
);

const { result } = renderHook(() =>
useActivityGraph({ baseUrl: "", token: "token", fetchImpl }),
);

await waitFor(() => expect(result.current.graph).not.toBeNull());
expect(result.current.replaceSchedule).toBeUndefined();
expect(result.current.deleteSchedule).toBeUndefined();
});

it("replaces the graph after a structural mutation", async () => {
const fetchImpl = vi
.fn()
Expand Down
10 changes: 10 additions & 0 deletions src/move37/services/activity_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,11 @@ def replace_schedule(
activity_id: str,
peers: list[SchedulePeer],
) -> dict[str, Any]:
"""Reject manual schedule replacement.

Schedule edges are derived from startDate by the deterministic planner.
This stub exists solely to back the legacy REST route with a clear error.
"""
del subject, activity_id, peers
raise ConflictError("Manual schedule rules are derived from startDate and cannot be edited directly.")

Expand All @@ -439,6 +444,11 @@ def delete_dependency(self, subject: str, parent_id: str, child_id: str) -> dict
return self._save_graph(subject, snapshot)

def delete_schedule(self, subject: str, earlier_id: str, later_id: str) -> dict[str, Any]:
"""Reject manual schedule deletion.

Schedule edges are derived from startDate by the deterministic planner.
This stub exists solely to back the legacy REST route with a clear error.
"""
del subject, earlier_id, later_id
raise ConflictError("Manual schedule rules are derived from startDate and cannot be deleted directly.")

Expand Down
Loading